[PATCH xserver] xfree86: add NoMatchFoo directives for InputClass sections
Benjamin Tissoires
benjamin.tissoires at gmail.com
Fri Dec 11 01:45:32 PST 2015
On Fri, Dec 11, 2015 at 10:42 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Fri, Dec 11, 2015 at 09:58:34AM +0100, Benjamin Tissoires wrote:
>> Hi Peter,
>>
>> On Fri, Dec 11, 2015 at 12:58 AM, Peter Hutterer
>> <peter.hutterer at who-t.net> wrote:
>> > InputClass sections use various MatchFoo directives to decide which device to
>> > apply to. This usually works fine for specific snippets but has drawbacks for
>> > snippets that apply more generally to a multitude of devices.
>> >
>> > This patch adds a NoMatchFoo directive to negate a match, thus allowing
>> > snippets that only apply if a given condition is not set. Specifically, this
>> > allows for more flexible fallback driver matching, it is now possible to use a
>> > snippet that says "assign driver foo, but only if driver bar wasn't already
>> > assigned to it". For example:
>> >
>> > Section "InputClass"
>> > Identifier "libinput for tablets"
>> > MatchIsTablet "true"
>> > NoMatchDriver "wacom"
>> > Driver "libinput"
>> > EndSection
>> >
>> > The above only assigns libinput to tablet devices if wacom isn't already
>> > assigned to this device, making it possible to select a specific driver by
>> > installing/uninstalling it.
>> >
>> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>
>> Thanks, that was quicker than expected :)
>>
>> Just 2 nitpicks/questions:
>>
>> > ---
>> > hw/xfree86/common/xf86Xinput.c | 15 +++++----
>> > hw/xfree86/man/xorg.conf.man | 15 +++++++++
>> > hw/xfree86/parser/InputClass.c | 76 ++++++++++++++++++++++++++++++++++++------
>> > hw/xfree86/parser/xf86Parser.h | 1 +
>> > hw/xfree86/parser/xf86tokens.h | 12 ++++++-
>> > 5 files changed, 102 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
>> > index c56a2b9..fead782 100644
>> > --- a/hw/xfree86/common/xf86Xinput.c
>> > +++ b/hw/xfree86/common/xf86Xinput.c
>> > @@ -540,21 +540,24 @@ MatchAttrToken(const char *attr, struct xorg_list *patterns,
>> > if (xorg_list_is_empty(patterns))
>> > return TRUE;
>> >
>> > - /* If there are patterns but no attribute, reject the match */
>> > - if (!attr)
>> > - return FALSE;
>> > -
>> > /*
>> > * Otherwise, iterate the list of patterns ensuring each entry has a
>>
>> If we remove the previous comment, we should probably remove the
>> "Otherwise" here.
>
> fixed locally, thanks.
>
>>
>> > * match. Each list entry is a separate Match line of the same type.
>> > */
>> > xorg_list_for_each_entry(group, patterns, entry) {
>> > char *const *cur;
>> > - Bool match = FALSE;
>> > + Bool is_negated = group->is_negated;
>> > + Bool match = is_negated;
>> > +
>> > + /* If there's a pattern but no attribute, we reject the match for a
>> > + * MatchFoo directive, and accept it for a NoMatchFoo directive
>> > + */
>>
>> I don't follow the logic here. Can you explain why a MatchFoo would be
>> rejected while a NoMatchFoo won't? I can't find an example for this
>> situation.
>
> If you have a snippet that says "Match on foo" and you have NULL, we reject
> because foo isn't NULL. If you have a snippet that says "Dont match on foo"
> and you have NULL, we accept because NULL isn't foo :)
>
> I hit this case with this snippet:
> Section "InputClass"
> NoMatchDriver "libinput"
> Driver "evdev"
> ...
> EndSection
>
> If libinput isn't installed and the udev backend doesn't set the driver
> (which it usually doesn't except on some old debian LTS systems), we would
> always reject the match and thus never assign a driver.
>
Oh I see my mistake now. I thought the 'attr' was the string
"libinput" in this example, while it is the currently stored attribute
for this particular matching pattern.
The patch is then:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>
Cheers,
Benjamin
>
>> > + if (!attr)
>> > + return is_negated;
>> >
>> > for (cur = group->values; *cur; cur++)
>> > if ((*compare) (attr, *cur) == 0) {
>> > - match = TRUE;
>> > + match = !is_negated;
>> > break;
>> > }
>> > if (!match)
>> > diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
>> > index 08eb7a9..d794df4 100644
>> > --- a/hw/xfree86/man/xorg.conf.man
>> > +++ b/hw/xfree86/man/xorg.conf.man
>> > @@ -1092,6 +1092,7 @@ attribute. For example:
>> > .B " # either gizmo or gadget
>> > .B " MatchProduct \*qexample\*q
>> > .B " MatchProduct \*qgizmo|gadget\*q
>> > +.B " NoMatchDriver \*qdrivername\*q
>> > .I " ..."
>> > .B "EndSection"
>> > .fi
>> > @@ -1160,6 +1161,20 @@ if no named
>> > .B ServerLayout
>> > sections have been found.
>> > .PP
>> > +The above directives have equivalents for negative matching with the
>> > +.B NoMatchProduct,
>> > +.B NoMatchVendor,
>> > +.B NoMatchDevicePath,
>> > +.B NoMatchOS,
>> > +.B NoMatchPnPID,
>> > +.B NoMatchUSBID,
>> > +.B NoMatchDriver,
>> > +.B NoMatchTag,
>> > +and
>> > +.B NoMatchLayout
>> > +directives. These NoMatch directives match if the subsequent match is not
>> > +met by the device.
>> > +.PP
>> > The second type of entry is used to match device types. These entries take a
>> > boolean argument similar to
>> > .B Option
>> > diff --git a/hw/xfree86/parser/InputClass.c b/hw/xfree86/parser/InputClass.c
>> > index 2b68aaa..5d3b59c 100644
>> > --- a/hw/xfree86/parser/InputClass.c
>> > +++ b/hw/xfree86/parser/InputClass.c
>> > @@ -55,6 +55,15 @@ xf86ConfigSymTabRec InputClassTab[] = {
>> > {MATCH_IS_TABLET, "matchistablet"},
>> > {MATCH_IS_TOUCHPAD, "matchistouchpad"},
>> > {MATCH_IS_TOUCHSCREEN, "matchistouchscreen"},
>> > + {NOMATCH_PRODUCT, "nomatchproduct"},
>> > + {NOMATCH_VENDOR, "nomatchvendor"},
>> > + {NOMATCH_DEVICE_PATH, "nomatchdevicepath"},
>> > + {NOMATCH_OS, "nomatchos"},
>> > + {NOMATCH_PNPID, "nomatchpnpid"},
>> > + {NOMATCH_USBID, "nomatchusbid"},
>> > + {NOMATCH_DRIVER, "nomatchdriver"},
>> > + {NOMATCH_TAG, "nomatchtag"},
>> > + {NOMATCH_LAYOUT, "nomatchlayout"},
>> > {-1, ""},
>> > };
>> >
>> > @@ -138,13 +147,19 @@ xf86freeInputClassList(XF86ConfInputClassPtr ptr)
>> >
>> > #define TOKEN_SEP "|"
>> >
>> > +enum MatchType {
>> > + MATCH_NORMAL,
>> > + MATCH_NEGATED,
>> > +};
>> > +
>> > static void
>> > -add_group_entry(struct xorg_list *head, char **values)
>> > +add_group_entry(struct xorg_list *head, char **values, enum MatchType type)
>> > {
>> > xf86MatchGroup *group;
>> >
>> > group = malloc(sizeof(*group));
>> > if (group) {
>> > + group->is_negated = (type == MATCH_NEGATED);
>> > group->values = values;
>> > xorg_list_add(&group->entry, head);
>> > }
>> > @@ -155,6 +170,7 @@ xf86parseInputClassSection(void)
>> > {
>> > int has_ident = FALSE;
>> > int token;
>> > + enum MatchType matchtype;
>> >
>> > parsePrologue(XF86ConfInputClassPtr, XF86ConfInputClassRec)
>> >
>> > @@ -170,6 +186,8 @@ xf86parseInputClassSection(void)
>> > xorg_list_init(&ptr->match_layout);
>> >
>> > while ((token = xf86getToken(InputClassTab)) != ENDSECTION) {
>> > + matchtype = MATCH_NORMAL;
>> > +
>> > switch (token) {
>> > case COMMENT:
>> > ptr->comment = xf86addComment(ptr->comment, xf86_lex_val.str);
>> > @@ -195,65 +213,103 @@ xf86parseInputClassSection(void)
>> > case OPTION:
>> > ptr->option_lst = xf86parseOption(ptr->option_lst);
>> > break;
>> > + case NOMATCH_PRODUCT:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_PRODUCT:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchProduct");
>> > add_group_entry(&ptr->match_product,
>> > - xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + xstrtokenize(xf86_lex_val.str, TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_VENDOR:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_VENDOR:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchVendor");
>> > add_group_entry(&ptr->match_vendor,
>> > - xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + xstrtokenize(xf86_lex_val.str, TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_DEVICE_PATH:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_DEVICE_PATH:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchDevicePath");
>> > add_group_entry(&ptr->match_device,
>> > - xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + xstrtokenize(xf86_lex_val.str, TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_OS:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_OS:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchOS");
>> > - add_group_entry(&ptr->match_os, xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + add_group_entry(&ptr->match_os, xstrtokenize(xf86_lex_val.str,
>> > + TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_PNPID:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_PNPID:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchPnPID");
>> > add_group_entry(&ptr->match_pnpid,
>> > - xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + xstrtokenize(xf86_lex_val.str, TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_USBID:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_USBID:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchUSBID");
>> > add_group_entry(&ptr->match_usbid,
>> > - xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + xstrtokenize(xf86_lex_val.str, TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_DRIVER:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_DRIVER:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchDriver");
>> > add_group_entry(&ptr->match_driver,
>> > - xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + xstrtokenize(xf86_lex_val.str, TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_TAG:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_TAG:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchTag");
>> > - add_group_entry(&ptr->match_tag, xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + add_group_entry(&ptr->match_tag, xstrtokenize(xf86_lex_val.str,
>> > + TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > + case NOMATCH_LAYOUT:
>> > + matchtype = MATCH_NEGATED;
>> > + /* fallthrough */
>> > case MATCH_LAYOUT:
>> > if (xf86getSubToken(&(ptr->comment)) != STRING)
>> > Error(QUOTE_MSG, "MatchLayout");
>> > add_group_entry(&ptr->match_layout,
>> > - xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
>> > + xstrtokenize(xf86_lex_val.str, TOKEN_SEP),
>> > + matchtype);
>> > free(xf86_lex_val.str);
>> > break;
>> > case MATCH_IS_KEYBOARD:
>> > diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h
>> > index b3a50e5..a038f9e 100644
>> > --- a/hw/xfree86/parser/xf86Parser.h
>> > +++ b/hw/xfree86/parser/xf86Parser.h
>> > @@ -306,6 +306,7 @@ typedef struct {
>> > typedef struct {
>> > struct xorg_list entry;
>> > char **values;
>> > + Bool is_negated;
>> > } xf86MatchGroup;
>> >
>> > typedef struct {
>> > diff --git a/hw/xfree86/parser/xf86tokens.h b/hw/xfree86/parser/xf86tokens.h
>> > index bbd6b90..f955af0 100644
>> > --- a/hw/xfree86/parser/xf86tokens.h
>> > +++ b/hw/xfree86/parser/xf86tokens.h
>> > @@ -286,7 +286,17 @@ typedef enum {
>> > MATCH_IS_JOYSTICK,
>> > MATCH_IS_TABLET,
>> > MATCH_IS_TOUCHPAD,
>> > - MATCH_IS_TOUCHSCREEN
>> > + MATCH_IS_TOUCHSCREEN,
>> > +
>> > + NOMATCH_PRODUCT,
>> > + NOMATCH_VENDOR,
>> > + NOMATCH_DEVICE_PATH,
>> > + NOMATCH_OS,
>> > + NOMATCH_PNPID,
>> > + NOMATCH_USBID,
>> > + NOMATCH_DRIVER,
>> > + NOMATCH_TAG,
>> > + NOMATCH_LAYOUT,
>> > } ParserTokens;
>> >
>> > #endif /* _xf86_tokens_h */
>> > --
>> > 2.5.0
>> >
More information about the xorg-devel
mailing list