On Mon, 2011-01-24 at 20:52 -0800, Marek Olšák wrote: > Let's assume there are two options with names such that one is a substring > of another. Previously, if we only specified the longer one as a debug option, > the shorter one would be considered specified as well (because of strstr). > This commit fixes it by checking that each option is surrounded by commas. > > (a regexp would be nicer, but this is not a performance critical code) > --- > src/gallium/auxiliary/util/u_debug.c | 39 > +++++++++++++++++++++++++++++++++- > 1 files changed, 38 insertions(+), 1 deletions(-) > > diff --git a/src/gallium/auxiliary/util/u_debug.c > b/src/gallium/auxiliary/util/u_debug.c > index 59b7613..8cf7660 100644 > --- a/src/gallium/auxiliary/util/u_debug.c > +++ b/src/gallium/auxiliary/util/u_debug.c > @@ -180,6 +180,43 @@ debug_get_num_option(const char *name, long dfault) > return result; > } > > +static boolean str_has_option(const char *str, const char *name) > +{ > + const char *substr; > + > + /* OPTION=all */ > + if (!util_strcmp(str, "all")) { > + return TRUE; > + } > + > + /* OPTION=name */ > + if (!util_strcmp(str, name)) { > + return TRUE; > + } > + > + substr = util_strstr(str, name); > + > + if (substr) { > + unsigned name_len = strlen(name); > + > + /* OPTION=name,... */ > + if (substr == str && substr[name_len] == ',') { > + return TRUE; > + } > + > + /* OPTION=...,name */ > + if (substr+name_len == str+strlen(str) && substr[-1] == ',') { > + return TRUE; > + } > + > + /* OPTION=...,name,... */ > + if (substr[-1] == ',' && substr[name_len] == ',') { > + return TRUE; > + } > + } > + > + return FALSE; > +}
Marek, The intention is good -- it would be nice to get options obeyed properly --, but this will fail to find "foo" in "OPTION=prefixfoosuffix,foo", so it's replacing a bug with another. I'd prefer we stop using strstr completely, and instead do: 1 - find the first ',' or '\0' in the string 2 - compare the previous characters with the option being searched, and return TRUE if matches 3 - if it was ',' go back to 1, but starting from character after ','. 4 - otherwise return FALSE It should be robust and almost the same amount of code. Jose > unsigned long > debug_get_flags_option(const char *name, > @@ -207,7 +244,7 @@ debug_get_flags_option(const char *name, > else { > result = 0; > while( flags->name ) { > - if (!util_strcmp(str, "all") || util_strstr(str, flags->name )) > + if (str_has_option(str, flags->name)) > result |= flags->value; > ++flags; > } _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev