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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZQixJquSFzcYrWKh@renaissance-vector>
Date: Mon, 18 Sep 2023 22:20:54 +0200
From: Andrea Claudi <aclaudi@...hat.com>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: netdev@...r.kernel.org, David Ahern <dsahern@...il.com>
Subject: Re: [PATCH iproute2-next v3] allow overriding color option in
 environment

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@...workplumber.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.