netdev - Re: [iproute PATCH v4] Make colored output configurable

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <46aa7a15-0317-10e4-0404-580b38fe2efb@gmail.com>
Date:   Thu, 16 Aug 2018 07:06:07 -0600
From:   David Ahern <dsahern@...il.com>
To:     Phil Sutter <phil@....cc>,
        Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org, Till Maas <opensource@...l.name>
Subject: Re: [iproute PATCH v4] Make colored output configurable

On 8/16/18 3:37 AM, Phil Sutter wrote:
> Allow for -color={never,auto,always} to have colored output disabled,
> enabled only if stdout is a terminal or enabled regardless of stdout
> state.
> 
> Signed-off-by: Phil Sutter <phil@....cc>
> ---
> Changes since v1:
> - Allow to override isatty() check by specifying '-color' flag more than
>   once.
> - Document new behaviour in man pages.
> 
> Changes since v2:
> - Implement new -color=foo syntax.
> - Update commit message and man page texts accordingly.
> 
> Changes since v3:
> - Fix typo in tc/tc.c causing compile error.
> ---
>  bridge/bridge.c   |  3 +--
>  include/color.h   |  7 +++++++
>  ip/ip.c           |  3 +--
>  lib/color.c       | 33 ++++++++++++++++++++++++++++++++-
>  man/man8/bridge.8 | 13 +++++++++++--
>  man/man8/ip.8     | 13 +++++++++++--
>  man/man8/tc.8     | 13 +++++++++++--
>  tc/tc.c           |  3 +--
>  8 files changed, 75 insertions(+), 13 deletions(-)
> 
> diff --git a/bridge/bridge.c b/bridge/bridge.c
> index 451d684e0bcfd..e35e5bdf7fb30 100644
> --- a/bridge/bridge.c
> +++ b/bridge/bridge.c
> @@ -173,8 +173,7 @@ main(int argc, char **argv)
>  			NEXT_ARG();
>  			if (netns_switch(argv[1]))
>  				exit(-1);
> -		} else if (matches(opt, "-color") == 0) {
> -			++color;
> +		} else if (matches_color(opt, &color) == 0) {
>  		} else if (matches(opt, "-compressvlans") == 0) {
>  			++compress_vlans;
>  		} else if (matches(opt, "-force") == 0) {
> diff --git a/include/color.h b/include/color.h
> index 4f2c918db7e43..42038dc2e7f87 100644
> --- a/include/color.h
> +++ b/include/color.h
> @@ -12,8 +12,15 @@ enum color_attr {
>  	COLOR_NONE
>  };
>  
> +enum color_opt {
> +	COLOR_OPT_NEVER = 0,
> +	COLOR_OPT_AUTO = 1,
> +	COLOR_OPT_ALWAYS = 2
> +};

The order of AUTO and ALWAYS is backwards. Existing users who do
something like:

    ip -c addr list | less -R
or
    ip -c addr list > /tmp/addr
    less -R /tmp/addr

should not be affected by this change. That is an existing command that
works and should continue to work the same after this change.

Users who add -c but don't want the codes if stdout is not a tty are the
ones who should be doing something new - be it adding another -c or
using -c=auto.