[PATCH 1/4] xserver: Use lists and constants for matching modes in Match entries
Dan Nicholson
dbn.lists at gmail.com
Wed Jun 8 10:39:40 PDT 2011
On Sat, Jun 4, 2011 at 10:48 PM, Oleh Nykyforchyn <oleh.nyk at gmail.com> wrote:
> Use lists and constants for matching modes in Match entries
You're leaving out some critical information here. Particularly that
each Match entry would take multiple arguments. I'm going to ignore
those details for now since I don't think we've agreed on it.
>
> Signed-off-by: Oleh Nykyforchyn <oleh.nyk at gmail.com>
> ---
> hw/xfree86/common/xf86Xinput.c | 117 +++++++-------
> hw/xfree86/parser/InputClass.c | 336 +++++++++++++++++++++++++---------------
> hw/xfree86/parser/xf86Parser.h | 23 +++-
> 3 files changed, 292 insertions(+), 184 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> index 26051ad..246108e 100644
> --- a/hw/xfree86/common/xf86Xinput.c
> +++ b/hw/xfree86/common/xf86Xinput.c
> @@ -443,77 +443,84 @@ HostOS(void)
> #endif
> }
>
> +/*
> + * Match an attribute against a pattern. Matching mode is
> + * determined by pattern->mode member.
> + */
> static int
> -match_substring(const char *attr, const char *pattern)
> +multi_match(const char *attr, xf86MatchPattern *pattern)
I like this function a lot now with the switch, but multi_match may
not be the best name. Maybe match_token? I'm also pretty sure pattern
could be const.
> {
> - return (strstr(attr, pattern)) ? 0 : -1;
> -}
> + if (!pattern) return 0;
I'm not sure others feel strongly about this, but I much prefer
expressions to be on a separate line from the conditional.
> + switch (pattern->mode)
> + {
> + case MATCH_IS_INVALID:
> + return 0;
> + case MATCH_IS_STRCMP:
> + return (strcmp(attr, pattern->str)) ? 0 : -1;
> + case MATCH_IS_STRCASECMP:
> + return (strcasecmp(attr, pattern->str)) ? 0 : -1;
> + /*
> + * If no Layout section is found, xf86ServerLayout.id becomes "(implicit)"
> + * It is convenient that "" in patterns means "no explicit layout"
> + */
> + case MATCH_IS_STRIMPLICIT:
> + if (*(pattern->str))
> + return (strcmp(attr, pattern->str)) ? 0 : -1;
> + else
> + return (strcmp(attr, "(implicit)")) ? 0 : -1;
> + case MATCH_IS_STRSTR:
> + return (strstr(attr, pattern->str)) ? -1 : 0;
> + case MATCH_IS_STRCASESTR:
> + return (strcasestr(attr, pattern->str)) ? -1 : 0;
> #ifdef HAVE_FNMATCH_H
> -static int
> -match_pattern(const char *attr, const char *pattern)
> -{
> - return fnmatch(pattern, attr, 0);
> -}
> + case MATCH_IS_FILENAME:
> + return (fnmatch(pattern->str, attr, 0)) ? 0 : -1;
> + case MATCH_IS_PATHNAME:
> + return (fnmatch(pattern->str, attr, FNM_PATHNAME)) ? 0 : -1;
> #else
> -#define match_pattern match_substring
> + case MATCH_IS_FILENAME:
> + return (strstr(attr, pattern->str)) ? -1 : 0;
> + case MATCH_IS_PATHNAME:
> + return (strstr(attr, pattern->str)) ? -1 : 0;
> #endif
> -
> -#ifdef HAVE_FNMATCH_H
> -static int
> -match_path_pattern(const char *attr, const char *pattern)
> -{
> - return fnmatch(pattern, attr, FNM_PATHNAME);
> -}
> -#else
> -#define match_path_pattern match_substring
> -#endif
> -
> -/*
> - * If no Layout section is found, xf86ServerLayout.id becomes "(implicit)"
> - * It is convenient that "" in patterns means "no explicit layout"
> - */
> -static int
> -match_string_implicit(const char *attr, const char *pattern)
> -{
> - if (strlen(pattern)) {
> - return strcmp(attr, pattern);
> - } else {
> - return strcmp(attr,"(implicit)");
> + default:
> + /* Impossible */
> }
> }
>
> /*
> - * Match an attribute against a list of NULL terminated arrays of patterns.
> - * If a pattern in each list entry is matched, return TRUE.
> + * Match an attribute against a list of xf86MatchGroup's.
> + * Return TRUE only if each list entry is successful.
> */
> static Bool
> -MatchAttrToken(const char *attr, struct list *patterns,
> - int (*compare)(const char *attr, const char *pattern))
> +MatchAttrToken(const char *attr, struct list *groups)
> {
> - const xf86MatchGroup *group;
> + xf86MatchGroup *group;
> + xf86MatchPattern *pattern;
>
> - /* If there are no patterns, accept the match */
> - if (list_is_empty(patterns))
> + /* If there are no groups, accept the match */
> + if (list_is_empty(groups))
> return TRUE;
>
> - /* If there are patterns but no attribute, reject the match */
> + /* If there are groups but no attribute, reject the match */
> if (!attr)
> return FALSE;
>
> /*
> - * Otherwise, iterate the list of patterns ensuring each entry has a
> - * match. Each list entry is a separate Match line of the same type.
> + * Otherwise, iterate the list of groups ensuring each entry has a
> + * match. Each list entry is a list of patterns obtained from
> + * a separate Match line.
> */
> - list_for_each_entry(group, patterns, entry) {
> - char * const *cur;
> + list_for_each_entry(group, groups, entry) {
> Bool match = FALSE;
>
> - for (cur = group->values; *cur; cur++)
> - if ((*compare)(attr, *cur) == 0) {
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + if (multi_match(attr, pattern)) {
> match = TRUE;
> break;
> }
> + }
> if (!match)
> return FALSE;
> }
> @@ -531,31 +538,31 @@ InputClassMatches(const XF86ConfInputClassPtr iclass, const InputInfoPtr idev,
> const InputAttributes *attrs)
> {
> /* MatchProduct substring */
> - if (!MatchAttrToken(attrs->product, &iclass->match_product, match_substring))
> + if (!MatchAttrToken(attrs->product, &iclass->match_product))
> return FALSE;
>
> /* MatchVendor substring */
> - if (!MatchAttrToken(attrs->vendor, &iclass->match_vendor, match_substring))
> + if (!MatchAttrToken(attrs->vendor, &iclass->match_vendor))
> return FALSE;
>
> /* MatchDevicePath pattern */
> - if (!MatchAttrToken(attrs->device, &iclass->match_device, match_path_pattern))
> + if (!MatchAttrToken(attrs->device, &iclass->match_device))
> return FALSE;
>
> /* MatchOS case-insensitive string */
> - if (!MatchAttrToken(HostOS(), &iclass->match_os, strcasecmp))
> + if (!MatchAttrToken(HostOS(), &iclass->match_os))
> return FALSE;
>
> /* MatchPnPID pattern */
> - if (!MatchAttrToken(attrs->pnp_id, &iclass->match_pnpid, match_pattern))
> + if (!MatchAttrToken(attrs->pnp_id, &iclass->match_pnpid))
> return FALSE;
>
> /* MatchUSBID pattern */
> - if (!MatchAttrToken(attrs->usb_id, &iclass->match_usbid, match_pattern))
> + if (!MatchAttrToken(attrs->usb_id, &iclass->match_usbid))
> return FALSE;
>
> /* MatchDriver string */
> - if (!MatchAttrToken(idev->driver, &iclass->match_driver, strcmp))
> + if (!MatchAttrToken(idev->driver, &iclass->match_driver))
> return FALSE;
>
> /*
> @@ -569,7 +576,7 @@ InputClassMatches(const XF86ConfInputClassPtr iclass, const InputInfoPtr idev,
> if (!attrs->tags)
> return FALSE;
> for (tag = attrs->tags, match = FALSE; *tag; tag++) {
> - if (MatchAttrToken(*tag, &iclass->match_tag, strcmp)) {
> + if (MatchAttrToken(*tag, &iclass->match_tag)) {
> match = TRUE;
> break;
> }
> @@ -579,9 +586,7 @@ InputClassMatches(const XF86ConfInputClassPtr iclass, const InputInfoPtr idev,
> }
>
> /* MatchLayout string */
> - if (!list_is_empty(&iclass->match_layout)) {
> - if (!MatchAttrToken(xf86ConfigLayout.id,
> - &iclass->match_layout, match_string_implicit))
> + if (!MatchAttrToken(xf86ConfigLayout.id, &iclass->match_layout))
> return FALSE;
> }
1. This brace appears to be hanging. Did this compile?
2. The handling of "(implicit)" with the new MATCH_IS_STRIMPLICIT
seems not very ideal. Instead of having a new type, why don't we just
sanitize xf86ConfigLayout.id before passing it to MatchAttrToken?
const char *layout;
if (strcmp(xf86ConfigLayout.id, "(implicit)") == 0)
layout = "";
else
layout = xf86ConfigLayout.id;
if (!MatchAttrToken(layout, &iclass->match_layout))
return FALSE;
And then MATCH_IS_STRCMP is can be used normally.
> diff --git a/hw/xfree86/parser/InputClass.c b/hw/xfree86/parser/InputClass.c
> index 3f80170..c896192 100644
> --- a/hw/xfree86/parser/InputClass.c
> +++ b/hw/xfree86/parser/InputClass.c
> @@ -64,18 +64,58 @@ xf86ConfigSymTabRec InputClassTab[] =
>
> #define CLEANUP xf86freeInputClassList
>
> -#define TOKEN_SEP "|"
> +#define TOKEN_SEP '|'
> +
>
> static void
> -add_group_entry(struct list *head, char **values)
> +add_to_group(xf86MatchGroup **group, const char *str,
> + xf86MatchMode pref_mode)
> {
> - xf86MatchGroup *group;
> + xf86MatchPattern *pattern;
> + unsigned n;
> + char *next;
> +
> + if (!str) return;
Same comment as above.
> - group = malloc(sizeof(*group));
> - if (group) {
> - group->values = values;
> - list_add(&group->entry, head);
> + if (!*group) {
> + *group = malloc(sizeof(**group));
> + if (!*group) return;
> + list_init(&(*group)->patterns);
> }
I think it would be a lot cleaner to initialize the group in
xf86parseInputClassSection instead of magically doing it here.
> +
> + again:
> + /* start new pattern */
> + if ((pattern = calloc(sizeof(*pattern),1)) == NULL)
> + return;
> + list_add(&pattern->entry, &(*group)->patterns);
> +
> + pattern->mode = pref_mode;
> +
> + if ((next = index(str, TOKEN_SEP)))
> + n = next-str;
> + else
> + n = strlen(str);
Why use this index loop when strtok will already do it for you? I'd
rather not start building a parser here.
> +
> + if ((pattern->str = strndup(str, n)) == NULL) {
> + pattern->mode = MATCH_IS_INVALID;
If strndup failed, I don't think calling it invalid is correct.
> + }
> +
> + if (next) {
> + str = next+1;
> + goto again;
> + }
> +}
> +
> +static void
> +free_group(xf86MatchGroup *group)
> +{
> + xf86MatchPattern *pattern, *next_pattern;
> + list_for_each_entry_safe(pattern, next_pattern, &group->patterns, entry) {
> + list_del(&pattern->entry);
> + if (pattern->str) free(pattern->str);
> + free(pattern);
> + }
> + free(group);
> }
>
> XF86ConfInputClassPtr
> @@ -83,6 +123,7 @@ xf86parseInputClassSection(void)
> {
> int has_ident = FALSE;
> int token;
> + xf86MatchGroup *group;
>
> parsePrologue(XF86ConfInputClassPtr, XF86ConfInputClassRec)
>
> @@ -98,6 +139,7 @@ xf86parseInputClassSection(void)
> list_init(&ptr->match_layout);
>
> while ((token = xf86getToken(InputClassTab)) != ENDSECTION) {
> + group = NULL;
> switch (token) {
> case COMMENT:
> ptr->comment = xf86addComment(ptr->comment, val.str);
> @@ -124,59 +166,95 @@ xf86parseInputClassSection(void)
> ptr->option_lst = xf86parseOption(ptr->option_lst);
> break;
> case MATCH_PRODUCT:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
Please add spaces around = and after ,. Makes the code much easier to read.
> + add_to_group(&group, val.str, MATCH_IS_STRSTR);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_product);
> + else
> Error(QUOTE_MSG, "MatchProduct");
> - add_group_entry(&ptr->match_product,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
Why did you change all the breaks to continues? If there was code
below the switch you'd be skipping it.
> case MATCH_VENDOR:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_STRSTR);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_vendor);
> + else
> Error(QUOTE_MSG, "MatchVendor");
> - add_group_entry(&ptr->match_vendor,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_DEVICE_PATH:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_PATHNAME);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_device);
> + else
> Error(QUOTE_MSG, "MatchDevicePath");
> - add_group_entry(&ptr->match_device,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_OS:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_STRCASECMP);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_os);
> + else
> Error(QUOTE_MSG, "MatchOS");
> - add_group_entry(&ptr->match_os,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_PNPID:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_FILENAME);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_pnpid);
> + else
> Error(QUOTE_MSG, "MatchPnPID");
> - add_group_entry(&ptr->match_pnpid,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_USBID:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_FILENAME);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_usbid);
> + else
> Error(QUOTE_MSG, "MatchUSBID");
> - add_group_entry(&ptr->match_usbid,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_DRIVER:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_STRCMP);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_driver);
> + else
> Error(QUOTE_MSG, "MatchDriver");
> - add_group_entry(&ptr->match_driver,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_TAG:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_STRCMP);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_tag);
> + else
> Error(QUOTE_MSG, "MatchTag");
> - add_group_entry(&ptr->match_tag,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_LAYOUT:
> - if (xf86getSubToken(&(ptr->comment)) != STRING)
> + while ((token=xf86getSubTokenWithTab(&(ptr->comment),InputClassTab)) == STRING) {
> + add_to_group(&group, val.str, MATCH_IS_STRIMPLICIT);
> + }
> + xf86unGetToken(token);
> + if (group)
> + list_add(&group->entry, &ptr->match_layout);
> + else
> Error(QUOTE_MSG, "MatchLayout");
> - add_group_entry(&ptr->match_layout,
> - xstrtokenize(val.str, TOKEN_SEP));
> - break;
> + continue;
> case MATCH_IS_KEYBOARD:
> if (xf86getSubToken(&(ptr->comment)) != STRING)
> Error(QUOTE_MSG, "MatchIsKeyboard");
> @@ -244,11 +322,25 @@ xf86parseInputClassSection(void)
> return ptr;
> }
>
> +static void
> +print_pattern(FILE * cf, const xf86MatchPattern *pattern)
> +{
> + if (!pattern) return;
> +
> + fprintf(cf, "\"");
> + if (pattern->mode == MATCH_IS_INVALID)
> + fprintf(cf, "invalid:");
> + if (pattern->str)
> + fprintf(cf, "%s\"", pattern->str);
> + else
> + fprintf(cf, "(none)\"");
> +}
> +
> void
> xf86printInputClassSection (FILE * cf, XF86ConfInputClassPtr ptr)
> {
> const xf86MatchGroup *group;
> - char * const *cur;
> + const xf86MatchPattern *pattern;
>
> while (ptr) {
> fprintf(cf, "Section \"InputClass\"\n");
> @@ -260,67 +352,76 @@ xf86printInputClassSection (FILE * cf, XF86ConfInputClassPtr ptr)
> fprintf(cf, "\tDriver \"%s\"\n", ptr->driver);
>
> list_for_each_entry(group, &ptr->match_product, entry) {
> - fprintf(cf, "\tMatchProduct \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchProduct ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_vendor, entry) {
> - fprintf(cf, "\tMatchVendor \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchVendor ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_device, entry) {
> - fprintf(cf, "\tMatchDevicePath \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchDevicePath ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_os, entry) {
> - fprintf(cf, "\tMatchOS \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchOS ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_pnpid, entry) {
> - fprintf(cf, "\tMatchPnPID \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchPnPID ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_usbid, entry) {
> - fprintf(cf, "\tMatchUSBID \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchUSBID ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_driver, entry) {
> - fprintf(cf, "\tMatchDriver \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchDriver ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_tag, entry) {
> - fprintf(cf, "\tMatchTag \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchTag ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
> list_for_each_entry(group, &ptr->match_layout, entry) {
> - fprintf(cf, "\tMatchLayout \"");
> - for (cur = group->values; *cur; cur++)
> - fprintf(cf, "%s%s", cur == group->values ? "" : TOKEN_SEP,
> - *cur);
> - fprintf(cf, "\"\n");
> + fprintf(cf, "\tMatchLayout ");
> + list_for_each_entry(pattern, &group->patterns, entry) {
> + fprintf(cf, " ");
> + print_pattern(cf, pattern);
> + }
> + fprintf(cf, "\n");
> }
>
> if (ptr->is_keyboard.set)
> @@ -353,65 +454,46 @@ xf86freeInputClassList (XF86ConfInputClassPtr ptr)
> XF86ConfInputClassPtr prev;
>
> while (ptr) {
> - xf86MatchGroup *group, *next;
> - char **list;
> + xf86MatchGroup *group, *next_group;
>
> TestFree(ptr->identifier);
> TestFree(ptr->driver);
>
> - list_for_each_entry_safe(group, next, &ptr->match_product, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_product, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_vendor, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_vendor, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_device, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_device, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_os, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_os, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_pnpid, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_pnpid, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_usbid, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_usbid, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_driver, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_driver, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_tag, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_tag, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
> - list_for_each_entry_safe(group, next, &ptr->match_layout, entry) {
> + list_for_each_entry_safe(group, next_group, &ptr->match_layout, entry) {
> list_del(&group->entry);
> - for (list = group->values; *list; list++)
> - free(*list);
> - free(group);
> + free_group(group);
> }
>
> TestFree(ptr->comment);
> diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h
> index a8785c5..629fe5e 100644
> --- a/hw/xfree86/parser/xf86Parser.h
> +++ b/hw/xfree86/parser/xf86Parser.h
> @@ -341,10 +341,31 @@ xf86TriState;
> typedef struct
> {
> struct list entry;
> - char **values;
> + struct list patterns;
> }
> xf86MatchGroup;
>
> +typedef enum
> +{
> + MATCH_IS_INVALID,
> + MATCH_IS_STRCMP,
> + MATCH_IS_STRCASECMP,
> + MATCH_IS_STRIMPLICIT,
> + MATCH_IS_STRSTR,
> + MATCH_IS_STRCASESTR,
> + MATCH_IS_FILENAME,
> + MATCH_IS_PATHNAME
> +}
> +xf86MatchMode;
> +
> +typedef struct
> +{
> + struct list entry;
> + xf86MatchMode mode;
> + char *str;
> +}
> +xf86MatchPattern;
Why do you need to add the list of patterns to xf86MatchGroup? It
seems like all that's needed is to add xf86MatchMode to xf86MatchGroup
in this patch.
--
Dan
More information about the xorg-devel
mailing list