[PATCH] regular expression support (was Re: [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn)
Oleh Nykyforchyn
oleh.nyk at gmail.com
Fri May 27 13:11:14 PDT 2011
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.
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"?
> 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,...}?
Oleh Nykyforchyn <oleh.nyk at gmail.com>
More information about the xorg-devel
mailing list