[PATCH xfree86] Signed-off-by: Oleh Nykyforchyn <oleh.nyk at gmail.com>

[Oleh R. Nykyforchyn]

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.

> 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. 

> > +                    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):
*** 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;
      }