On Fri, 27 May 2011 05:51:13 -0700 Dan Nicholson <[email protected]> wrote:
> On Thu, May 26, 2011 at 10:56 PM, Peter Hutterer > <[email protected]> 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 <[email protected]> _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
