xfree86: allow negative conditions in "Match*" statements (was [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn <oleh.nyk at gmail.com>)

[Peter Hutterer]

On Tue, May 17, 2011 at 01:27:30PM +0300, Oleh R. Nykyforchyn wrote:
> On Tue, 17 May 2011 16:14:03 +1000
> Peter Hutterer <peter.hutterer at who-t.net> wrote:
> 
> > On Mon, May 16, 2011 at 09:15:51PM +0300, Oleh R. Nykyforchyn wrote:
> > > xfree86: allow negative conditions in "Match*" statements
> > > 
> > > Match statement syntax is extended to allow strings like:
> > > "aaa,!a,bbb,!b,ccc,!c"
> > > Match succeedes if an attribute matches aaa, bbb, or ccc, or
> > > does not match neither a, b, or c.
> > > 
> > > Signed-off-by: Oleh Nykyforchyn <oleh.nyk at gmail.com>
> > 
> > this needs an entry in the man page. 
> I am not a native English speaker. I can write some "raw" text, but it
> will need to be revised by someone with better English skills.

Our English requirements for man pages are surprisingly low ;)
Given your emails it'll be easy to pop out a decent man page entry.

> > I also wonder how long it will take us
> > until we need full regex support...
> I am not sure that we will ever need it. Smth like "fnmatch" is well enough.
> 
> > 
> > do all handlers handle NULL as pattern argument? if not, we need an extra
> > condition here.
> I looked through the code of xfree86/{common,parser} and found no way for
> NULL to get into (*compare). Such a check has not been there before. 

sorry, nevermind that. I misread the type of cur.

> 
> > > +                    return FALSE;
> > 
> > break instead?
> 
> No, it is possible that match==TRUE because some previous negative match was successful.
> Nevertheless, if this negative match fail, we should fail as well.
> > > +            else {
> > > +                if ((*compare)(attr, *cur) == 0) {
> > 
> > else if ...  on one line please
> > 
> > 
> > > +        if (!match) return FALSE;
> > 
> > two lines please
> Done.
> 
> > 
> > I wonder if this flow could be made simpler with a few temporary variables.
> > 
> > Bool positive_match = TRUE;
> > Bool match_result = FALSE;
> > 
> > match_result = (*compare)(...)
> > 
> > return (positive_match) ?  match_result : !match_result;
> 
> Not sure, because we treat positive and negative conditions differently, see above.
> 
> Here is a result (simple diff -c):

urgh. git diff please next time, i'm really not used to this format anymore.
but yeah, makes sense now. thank you.
pop the man page stuff in and I'll merge it in.

Cheers,
  Peter

> *** xf86Xinput.c.orig	2011-05-16 20:11:27.000000000 +0300
> --- xf86Xinput.c	2011-05-17 13:11:51.000000000 +0300
> ***************
> *** 495,505 ****
>           char * const *cur;
>           Bool match = FALSE;
>   
> !         for (cur = group->values; *cur; cur++)
> !             if ((*compare)(attr, *cur) == 0) {
> !                 match = TRUE;
> !                 break;
>               }
>           if (!match)
>               return FALSE;
>       }
> --- 495,521 ----
>           char * const *cur;
>           Bool match = FALSE;
>   
> !         for (cur = group->values; *cur; cur++) {
> !             if (**cur == '!') {
> !             /*
> !              * A condition starting with '!' is NEGATIVE
> !              * If it is matched, the match is rejected
> !              */
> !                 if ((*compare)(attr, *cur+1) == 0)
> !                     return FALSE;
> !                 else
> !                     match = TRUE;
>               }
> +             else if ((*compare)(attr, *cur) == 0) {
> +                     match = TRUE;
> +                     break;
> +             }
> +         }
> +         /*
> +          * Hence we accept the match only if either a positive condition
> +          * and all previous negative conditions succeed, or ALL (<>0)
> +          * negative conditions are successful (i.e. not matched)
> +          */
>           if (!match)
>               return FALSE;
>       }