On Thu, Nov 19, 2015 at 5:31 AM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> Hi Rob,
>
> On 18.11.2015 23:20, Rob Clark wrote:
>>
>> From: Rob Clark <robcl...@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 <robcl...@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 */
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
