[PATCH 2/3 v2'] xserver: Introduce negated conditions in Match patterns

[Dan Nicholson]

On Fri, Jun 17, 2011 at 6:57 AM, Oleh Nykyforchyn <oleh.nyk at gmail.com> wrote:
> xserver: Introduce negated conditions in Match patterns
>
> When processing input devices, it is useful to determine not only which
> attributes are to be accepted, but also which are to be rejected. Hence we introduce
> an '!' prefix, before a pattern, which means that if an attribute matches this
> pattern, then the respective device is rejected by this InputClass. To allow
> statements like
>    MatchVendor "!Asus|!Logitech"
> we also accept the rule of "last pattern": if the end of an entry is reached,
> then the last pattern is decisive.
>
> Signed-off-by: Oleh Nykyforchyn <oleh.nyk at gmail.com>
> ---
>  hw/xfree86/common/xf86Xinput.c |   15 ++++++++++++---
>  hw/xfree86/man/xorg.conf.man   |   19 ++++++++++++-------
>  hw/xfree86/parser/InputClass.c |   10 ++++++++++
>  hw/xfree86/parser/xf86Parser.h |    1 +
>  4 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> index 22e58a5..2cd1508 100644
> --- a/hw/xfree86/common/xf86Xinput.c
> +++ b/hw/xfree86/common/xf86Xinput.c
> @@ -509,10 +509,19 @@ MatchAttrToken(const char *attr, struct list *groups)
>         Bool match = FALSE;
>
>         list_for_each_entry(pattern, &group->patterns, entry) {
> -            if (match_token(attr, pattern)) {
> -                match = TRUE;
> -                break;
> +            match = match_token(attr, pattern);
> +            if (match) {                  /* success? */
> +                if (pattern->is_negated)  /* negated pattern matched */
> +                    match = FALSE;        /* failure */
> +                break;                    /* leave the entry */
> +            }
> +            else {
> +                if (pattern->is_negated)  /* at least negated pattern not */
> +                    match = TRUE;         /*   matched - partial success */
> +                /* else non-negated pattern not matched */
> +                /*  match = FALSE;         */
>             }
> +             /* no success yet, continue */

This is really confusing. I think it says that if the pattern didn't
match the attribute and the pattern was preceded by !, then we keep
going even though it successfully matched what we asked for. Is this
the "last pattern" thing? Why don't we break when match = TRUE?

Assuming I've understood this correctly, I think it breaks how the
current match rules are supposed to work.

MatchProduct "foo|bar"
If the product name matches "foo" or "bar", use the class.

MatchProduct "foo"
MatchProduct "bar"
If the product name matches "foo" and "bar", use the class.

Following that logic, I would expect this:

MatchProduct "!foo|!bar"
If the product name does not match "foo" or does not match "bar", use the class.

MatchProduct "!foo"
MatchProduct "!bar"
If the product name does not match "foo" and does not match "bar", use
the class.

I think you're trying to make the former act like the latter because
that's what you'd typically want to do with negated conditions. Rather
than making the code goofy, I'd rather have people use the correct
logic. I think it could be really simple:

match = match_token(attr, pattern);
if ((match && !pattern->is_negated) ||
    (!match && pattern->is_negated)) {
    match = TRUE:
    break;
} else
    match = FALSE;

Or even simpler:

if (match_token(attr, pattern) == !pattern->is_negated) {
    match = TRUE:
    break;
}

--
Dan