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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <0f990b11-7fd9-4a56-2710-5ba8b62fd5f9@gmail.com>
Date: Wed, 20 Sep 2023 19:06:26 -0600
From: David Ahern <dsahern@...il.com>
To: Andrea Claudi <aclaudi@...hat.com>,
 Stephen Hemminger <stephen@...workplumber.org>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH iproute2-next v3] allow overriding color option in
 environment

On 9/18/23 2:20 PM, Andrea Claudi wrote:
> 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.
> 

FYI: I did not get this patch, and it is not showing in patchworks either.