<div class="gmail_quote">On Wed, Jan 26, 2011 at 12:17 PM, José Fonseca <<a href="mailto:jfonseca@vmware.com">jfonseca@vmware.com</a>> wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> <div><div></div><div class="h5">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; > +} </div></div>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. </blockquote><div> Alright. I'll fix it. Marek </div></div>