Subject: Re: bug#24232: ls --color cannot be interrupted by a signal

From: Pádraig Brady <[email-masked]>
Date: Mon, 15 Aug 2016 18:40:00 +0100

On 15/08/16 16:52, Kamil Dudka wrote:
> On Monday, August 15, 2016 15:55:13 Pádraig Brady wrote:
>> The signal catching functionality originated trying to restore terminal
>> color:
>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v4.5.3-89-g854
>> 9068
>>
>> That was adjusted to only outputting reset chars once for multiple signals,
>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-357-geae
>> 1b7f
>>
>> and not outputting reset chars at arbitrary places as that messes up
>> multi-byte chars:
>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.2.1-368-gad
>> c30a8
>>
>> Maybe we should just buffer internally at put_indicator
>> (&color_indicator[C_LEFT]); and output only at put_indicator
>> (&color_indicator[C_RIGHT]) or equivalent. That wouldn't introduce
>> significant overhead I think.
> 
> Internal buffering is certainly doable.  I guess there is some gnulib module 
> that already implements such a buffer?

Not that I noticed. The conditions for flushing tend to vary per app,
so we've slightly different variations in various utils, like lbuf_flush()
in factor(1), and buffer_or_output() in relpath(1).
We could take advantage of a PATH_MAX+A_BIT sized buffer to avoid realloc
complexities.

Actually we already buffer in quote_name(), so rather than outputting
the color sequences separately in print_name_with_quoting(), we could
pass those into quote_name() and it could write to its buffer,
and output the START+name+END atomically.

> However, what to do with signal handling then?  Drop the SA_RESTART flag and 
> implement EINTR loop only for the fwrite() call that writes data with escape 
> sequences to the terminal?

Hrm, the buffering above would avoid needing to explicitly output the color reset
sequence upon exit, but it would not avoid the need to fflush(), since stdio
might actually output the start and end color sequences in separate writes.
Drats, so if we had to manage signals anyway to do the fflush() the buffering
probably isn't worth it.

I'm not sure how to improve this.
Perhaps we should look at only enabling the signal handling before printing,
in the common case of outputting the sorted entries of a single directory.
In that way the open(), stat() etc. would be done with standard signal handling.