netdev - Re: [iproute PATCH 4/4] lib: Enable colored output only for TTYs

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: <20180815080406.7110bd76@xeon-e3>
Date:   Wed, 15 Aug 2018 08:04:06 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     David Ahern <dsahern@...il.com>
Cc:     Phil Sutter <phil@....cc>, netdev@...r.kernel.org,
        Till Maas <opensource@...l.name>
Subject: Re: [iproute PATCH 4/4] lib: Enable colored output only for TTYs

On Wed, 15 Aug 2018 08:40:20 -0600
David Ahern <dsahern@...il.com> wrote:

> On 8/15/18 3:06 AM, Phil Sutter wrote:
> > Add an additional prerequisite to check_enable_color() to make sure
> > stdout actually points to an open TTY device. Otherwise calls like
> > 
> > | ip -color a s >/tmp/foo
> > 
> > will print color escape sequences into that file.
> > 
> > Signed-off-by: Phil Sutter <phil@....cc>
> > ---
> >  lib/color.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/color.c b/lib/color.c
> > index edf96e5c6ecd7..500ba09682697 100644
> > --- a/lib/color.c
> > +++ b/lib/color.c
> > @@ -3,6 +3,7 @@
> >  #include <stdarg.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > +#include <unistd.h>
> >  #include <sys/socket.h>
> >  #include <sys/types.h>
> >  #include <linux/if.h>
> > @@ -79,7 +80,7 @@ void enable_color(void)
> >  
> >  int check_enable_color(int color, int json)
> >  {
> > -	if (color && !json) {
> > +	if (color && !json && isatty(fileno(stdout))) {
> >  		enable_color();
> >  		return 0;
> >  	}
> >   
> 
> This also disables color sequence when the output is piped to a pager
> such as less which with the -R argument can handle it just fine.
> 
> ie., the user needs to remove the color arg when that output is not wanted.

If you are going to change the color enabling, why not make it compatible
with what ls does?

From man ls(1) (and  grep)

       --color[=WHEN]
              colorize  the output; WHEN can be 'always' (default if omitted),
              'auto', or 'never'; more info below
...

       Using  color  to distinguish file types is disabled both by default and
       with --color=never.  With --color=auto, ls emits color codes only  when
       standard  output is connected to a terminal.  The LS_COLORS environment
       variable can change the settings.  Use the dircolors command to set it.


Make -c be same as --color=always to keep compatiablity