GNU bug report logs - #20062
[PATCH] diff: add support for --color

Previous Next

Full log

Message #23 received at 20062 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Giuseppe Scrivano <gscrivan <at> redhat.com>
Cc: Eric Blake <eblake <at> redhat.com>, 20062 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support
 for --color
Date: Tue, 10 Mar 2015 15:59:14 -0700

Giuseppe Scrivano wrote:

> +  sigemptyset (&set);
> +  for (j = 0; j < (sizeof (sig) / sizeof (*sig)); j++)
> +    sigaddset (&set, sig[j]);

Shouldn't the above part be done just once?  Also, it has redundant parentheses.

> +      sigprocmask (SIG_BLOCK, &set, &old_sigproc_set);

The sigprocmask module needs to be added to bootstrap.conf.

Can the proposed implementation block signals for an unbounded amount of time? 
That would be bad.  ls.c attempts to avoid this problem, and 'diff' should too.

Also, 'diff --color' needn't mess with signal handling unless isatty (fileno 
(outfile)).

Also, surely --color is incompatible with --paginate, in the sense that signals 
arriving while paginating will put outfile into a weird state if outfile is a 
tty, so I expect that combination of options should be rejected.

> What about amending the previous version with the patch below?

Yes, we do need the patch to be amended something along these lines.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.