[PATCH xserver] xfree86: add NoMatchFoo directives for InputClass sections
Peter Hutterer
peter.hutterer at who-t.net
Fri Dec 11 01:42:17 PST 2015
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.
Cheers,
Peter
> > + 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