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

[Dan Nicholson]

On Wed, Jan 26, 2011 at 3:17 AM, 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.

You could also strtok pretty easily, but it can be a little
heavyweight if you're doing this repeatedly.

char *tmp, *tok;

tmp = strdup(str);
for (tok = strtok(tmp, ","); tok; tok = strtok(NULL, ",")) {
    if (strcmp(tok, opt) == 0) {
        free(tmp);
        return TRUE;
    }
}

free(tmp);
return FALSE;

--
Dan