[PATCH] regular expression support (was Re: [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn)

[Peter Hutterer]

On Fri, May 27, 2011 at 11:11:14PM +0300, Oleh Nykyforchyn wrote:
> On Fri, 27 May 2011 05:51:13 -0700
> Dan Nicholson <dbn.lists at gmail.com> wrote:
> 
> > On Thu, May 26, 2011 at 10:56 PM, Peter Hutterer
> > <peter.hutterer at who-t.net> wrote:
> 
> > >>
> > >> +        cur = group->values;
> > >> +        if ((cur[0]) && (!*cur[0]) && /* cur[0] == "", can never come out of strtok() */
> > >> +            (cur[1]) &&               /* cur[1] == regex */
> > >> +            (cur[2] == NULL)) {
> > >
> > > urgh, this seems a bit hacked on. any chance we can have a define, enum,
> > > special return value so we don't rely on magic value combinations?
> > >
> > > patterns is a xf86MatchGroup struct, we could extend this with a enum type {
> > > } or somesuch.
> It depends on whether we are going to extend this choice of matching mode further.
> I am sure that intoduction of MATCH_TYPE_STRCMP, MATCH_TYPE_STRCASECMP etc it is
> useless because anyway this mode is applies to ALL patterns in the list. What we shall
> do if somebody wants to apply strcmp to the first pattern and fnmatch to the second one? 
> 
> Hence it is optimal to have two modes forever: "natural" for most cases and regex for rare
> complicated setups.

I'm not sure whether we can claim that regex are the rare complicated
setups. All it takes is one forum entry of someone using a regex and you'll
see it duplicated by the thousands.

but other than that, i hope that with the natural matching and regex
matching we have covered everything that'll be necessary in the future. and
while it may not be pretty, regex should match more cases than any of
strcmp, fnmatch, etc. together.

> If we fix forever that the choice {MATCH_TYPE_NORMAL, MATCH_TYPE_REGEX} is exhaustive,
> then this little bit of hackery is a least evil.
> > >
> > > Dan, any comments?
> > 
> > I think at this point I've made myself pretty clear what I think about
> > this idea. I don't think I need to repeat myself again.
> > 
> > For the actual code, I would agree that extending the xf86MatchGroup
> > with a type member would be better than strangely manipulating the
> > array. It's also going to be a big performance drain to compile the
> > regex every time though the matching code.
> Compare computations needed to compile a regex e.g. four times for four keyboard
> devices ONCE at the startup with resources necessary to paint 50x50 pixels 24 bits
> per pixel for EACH frame. Is this really a "big performance drain"?

do we have numbers or are we all just guessing here?

> > It would require more
> > surgery, but it would be better to compile the regexes once at parsing
> > time. By adding a type member to xf86MatchGroup and making the values
> > member void**, you can just cast to what's necessary in
> > MatchAttrToken.
> > 
> Now imagine that we use an
> union {
> enum MatchMode type;
> regex_t r;
> char **list;
> ...
> }
> Remember also that we are to free regex_t properly.
> 
> Is this better and clearer indeed?
> 
> And, finally, is there a big difference between
>     MatchProduct "re:^foo|bar$"
> and
>     MatchProduct "^foo|bar$" "regex"
> to add test for an optional argument to EACH peace
> of code for Match{Product,Layout,Vendor,...}?

IMO the second one looks cleaner and is harder to be confused. Adding that
optional argument shouldn't be a big deal.

Cheers,
  Peter