[Mesa-dev] [PATCH] util: require debug options to be separated by commas

[Marek Olšák]

On Wed, Jan 26, 2011 at 12:17 PM, José Fonseca <jfonseca at vmware.com> wrote:

> 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.
>

Alright. I'll fix it.

Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110127/1d8252ff/attachment.htm>