Re: [PATCH iproute2-next v3] allow overriding color option in environment

[Andrea Claudi <aclaudi@redhat.com>]

On Mon, Sep 18, 2023 at 08:29:10AM -0700, Stephen Hemminger wrote:
> For ip, tc, and bridge command introduce IPROUTE_COLORS to enable
> automatic colorization via environment variable.
> Similar to how grep handles color flag.
> 
> Example:
>   $ IPROUTE_COLORS=auto ip -br addr
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v3 - drop unneccessary check for NULL in match_colors
>      all three callers pass valid pointer.
>      drop unnecessary check for NULL in default_color

The NULL check in default_color is necessary, because getenv may return
NULL if there is no env variable with the desired name. Indeed it seems
to me this check is maintained in this patch.

However the null string check in default_color is also necessary
because, as I pointed out in the review of the RFC version of this
patch:

IPROUTE_COLORS= ip address

results in colorized output, while I would expect it to produce
colorless output.

This happens because we are effectively passing a null string to
default_color using the above syntax, and match_color_value() treat the
null string as 'always'.

Please note that this is indeed correct when calling match_color_value
when the '-c / --color' option is provided, but not when the env
variable is used to determine the color.