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

[Nicolai Hähnle]

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.

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

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 */
>