[PATCH 06/11] xfree86: Refactor InputClass matching code

[Jamey Sharp]

Cool, that's a nice refactoring. In MatchAttrToken you could go a
little further even, replacing "match = TRUE" with "return TRUE" and
"return match" with "return FALSE", but either way,
Reviewed-by: Jamey Sharp <jamey at minilop.net>

On Thu, May 20, 2010 at 7:09 AM, Dan Nicholson <dbn.lists at gmail.com> wrote:
> InputClassMatches was starting to get a little hairy with all the loops
> over the tokenized match strings. This adds code, but makes it easier to
> read and add new matches.
>
> Signed-off-by: Dan Nicholson <dbn.lists at gmail.com>
> ---
>  hw/xfree86/common/xf86Xinput.c |  121 +++++++++++++++++++++++-----------------
>  1 files changed, 70 insertions(+), 51 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> index 3f1de8f..d72150b 100644
> --- a/hw/xfree86/common/xf86Xinput.c
> +++ b/hw/xfree86/common/xf86Xinput.c
> @@ -496,74 +496,92 @@ AddOtherInputDevices(void)
>  {
>  }
>
> +static int
> +MatchSubstring(const char *attr, const char *pattern)
> +{
> +    return (strstr(pattern, attr)) ? 0 : -1;
> +}
> +
> +#ifdef HAVE_FNMATCH_H
> +static int
> +MatchPathPattern(const char *attr, const char *pattern)
> +{
> +    return fnmatch(pattern, attr, FNM_PATHNAME);
> +}
> +#else
> +#define MatchPathPattern MatchSubstring
> +#endif
> +
>  /*
> - * Classes without any Match statements match all devices. Otherwise, all
> - * statements must match.
> + * Match an attribute against a NULL terminated list of patterns. If any
> + * pattern is matched, return TRUE.
>  */
>  static Bool
> -InputClassMatches(XF86ConfInputClassPtr iclass, InputAttributes *attrs)
> +MatchAttrToken(const char *attr, char **patterns,
> +               int (*compare)(const char *attr, const char *pattern))
>  {
>     char **cur;
>     Bool match;
>
> -    if (iclass->match_product) {
> -        if (!attrs->product)
> -            return FALSE;
> -        /* see if any of the values match */
> -        for (cur = iclass->match_product, match = FALSE; *cur; cur++)
> -            if (strstr(attrs->product, *cur)) {
> -                match = TRUE;
> -                break;
> -            }
> -        if (!match)
> -            return FALSE;
> -    }
> -    if (iclass->match_vendor) {
> -        if (!attrs->vendor)
> -            return FALSE;
> -        /* see if any of the values match */
> -        for (cur = iclass->match_vendor, match = FALSE; *cur; cur++)
> -            if (strstr(attrs->vendor, *cur)) {
> -                match = TRUE;
> -                break;
> -            }
> -        if (!match)
> -            return FALSE;
> -    }
> -    if (iclass->match_device) {
> -        if (!attrs->device)
> -            return FALSE;
> -        /* see if any of the values match */
> -        for (cur = iclass->match_device, match = FALSE; *cur; cur++)
> -#ifdef HAVE_FNMATCH_H
> -            if (fnmatch(*cur, attrs->device, FNM_PATHNAME) == 0) {
> -#else
> -            if (strstr(attrs->device, *cur)) {
> -#endif
> -                match = TRUE;
> -                break;
> -            }
> -        if (!match)
> -            return FALSE;
> +    /* If there are no patterns, accept the match */
> +    if (!patterns)
> +        return TRUE;
> +
> +    /* If there are patterns but no attribute, reject the match */
> +    if (!attr)
> +        return FALSE;
> +
> +    /* Otherwise, iterate the patterns looking for a match */
> +    match = FALSE;
> +    for (cur = patterns; *cur; cur++) {
> +        if ((*compare)(attr, *cur) == 0) {
> +            match = TRUE;
> +            break;
> +        }
>     }
> +    return match;
> +}
> +
> +/*
> + * Classes without any Match statements match all devices. Otherwise, all
> + * statements must match.
> + */
> +static Bool
> +InputClassMatches(XF86ConfInputClassPtr iclass, InputAttributes *attrs)
> +{
> +    /* MatchProduct substring */
> +    if (!MatchAttrToken(attrs->product, iclass->match_product, MatchSubstring))
> +        return FALSE;
> +
> +    /* MatchVendor substring */
> +    if (!MatchAttrToken(attrs->vendor, iclass->match_vendor, MatchSubstring))
> +        return FALSE;
> +
> +    /* MatchDevicePath pattern */
> +    if (!MatchAttrToken(attrs->device, iclass->match_device, MatchPathPattern))
> +        return FALSE;
> +
> +    /*
> +     * MatchTag string
> +     * See if any of the device's tags match any of the MatchTag tokens.
> +     */
>     if (iclass->match_tag) {
> +        const char * const *tag;
> +        Bool match;
> +
>         if (!attrs->tags)
>             return FALSE;
> -
> -        for (cur = iclass->match_tag, match = FALSE; *cur && !match; cur++) {
> -            const char * const *tag;
> -            for(tag = attrs->tags; *tag; tag++) {
> -                if (!strcmp(*tag, *cur)) {
> -                    match = TRUE;
> -                    break;
> -                }
> +        for (tag = attrs->tags, match = FALSE; *tag; tag++) {
> +            if (MatchAttrToken(*tag, iclass->match_tag, strcmp)) {
> +                match = TRUE;
> +                break;
>             }
>         }
> -
>         if (!match)
>             return FALSE;
>     }
>
> +    /* MatchIs* booleans */
>     if (iclass->is_keyboard.set &&
>         iclass->is_keyboard.val != !!(attrs->flags & ATTR_KEYBOARD))
>         return FALSE;
> @@ -582,6 +600,7 @@ InputClassMatches(XF86ConfInputClassPtr iclass, InputAttributes *attrs)
>     if (iclass->is_touchscreen.set &&
>         iclass->is_touchscreen.val != !!(attrs->flags & ATTR_TOUCHSCREEN))
>         return FALSE;
> +
>     return TRUE;
>  }
>
> --
> 1.6.6.1
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>