[Mesa-dev] [PATCH 1/2] util: move brw_env_var_as_boolean() to util

[Rob Clark]

On Thu, Nov 19, 2015 at 5:31 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> Hi Rob,
>
> On 18.11.2015 23:20, Rob Clark wrote:
>>
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Kind of a handy function.  And I'll what it available outside of i965
>> for common nir-pass helpers.
>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>   src/mesa/drivers/dri/i965/brw_context.c  |  5 +++--
>>   src/mesa/drivers/dri/i965/brw_nir.c      |  4 +++-
>>   src/mesa/drivers/dri/i965/brw_shader.cpp |  3 ++-
>>   src/mesa/drivers/dri/i965/intel_debug.c  | 25 -------------------------
>>   src/mesa/drivers/dri/i965/intel_debug.h  |  2 --
>>   src/util/debug.c                         | 25 +++++++++++++++++++++++++
>>   src/util/debug.h                         |  2 ++
>>   7 files changed, 35 insertions(+), 31 deletions(-)
>
> [.. snip ...]
>
>> diff --git a/src/util/debug.c b/src/util/debug.c
>> index 3729ce8..98b1853 100644
>> --- a/src/util/debug.c
>> +++ b/src/util/debug.c
>> @@ -51,3 +51,28 @@ parse_debug_string(const char *debug,
>>
>>      return flag;
>>   }
>> +
>> +/**
>> + * Reads an environment variable and interprets its value as a boolean.
>> + *
>> + * Recognizes 0/false/no and 1/true/yes.  Other values result in the
>> default value.
>> + */
>> +bool
>> +env_var_as_boolean(const char *var_name, bool default_value)
>> +{
>> +   const char *str = getenv(var_name);
>> +   if (str == NULL)
>> +      return default_value;
>> +
>> +   if (strcmp(str, "1") == 0 ||
>> +       strcasecmp(str, "true") == 0 ||
>> +       strcasecmp(str, "yes") == 0) {
>> +      return true;
>> +   } else if (strcmp(str, "0") == 0 ||
>> +              strcasecmp(str, "false") == 0 ||
>> +              strcasecmp(str, "no") == 0) {
>> +      return false;
>> +   } else {
>> +      return default_value;
>> +   }
>> +}
>
>
> This all looks good to me. I do have two suggestions to slightly improve
> usability:
>
> 1) Add "on" and "off" as recognized values.
>
> 2) Add something to the effect of `fprintf(stderr, "%s: value not
> recognized, using default.\n", var_name);` to the default value branch.

Those would be reasonable additions, although should be split out into
a different patch..  I prefer to avoid making unrelated changes at
same time I move code around

> Either way, feel free to add my R-b.

Thanks

BR,
-R

> Cheers,
> Nicolai
>
>
>> diff --git a/src/util/debug.h b/src/util/debug.h
>> index 801736a..3555417 100644
>> --- a/src/util/debug.h
>> +++ b/src/util/debug.h
>> @@ -38,6 +38,8 @@ struct debug_control {
>>   uint64_t
>>   parse_debug_string(const char *debug,
>>                      const struct debug_control *control);
>> +bool
>> +env_var_as_boolean(const char *var_name, bool default_value);
>>
>>   #ifdef __cplusplus
>>   } /* extern C */
>>
>