Subject: bug#20062: [bug-diffutils] bug#20062: bug#20062: [PATCH] diff: add support for --color

From: Paul Eggert <[email-masked]>
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.