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: <20180816150035.GZ32448@orbyte.nwl.cc>
Date:   Thu, 16 Aug 2018 17:00:35 +0200
From:   Phil Sutter <phil@....cc>
To:     David Ahern <dsahern@...il.com>
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        netdev@...r.kernel.org, Till Maas <opensource@...l.name>
Subject: Re: [iproute PATCH v4] Make colored output configurable

On Thu, Aug 16, 2018 at 07:06:07AM -0600, David Ahern wrote:
> 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:

Ordering color_opt this way felt natural since the default (0)
automatically matches the existing default (no colors).

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

My code is correct in that regard: Plain -c[olor] is equivalent to
-color=always.

Cheers, Phil