Subject: Re: bug#10243: 8.14: ls --color is uninterruptible with ctrl+c (and no network fs in use)

From: Eric Blake <[email-masked]>
Date: Wed, 07 Dec 2011 16:16:49 -0700

On 12/07/2011 03:04 PM, Jim Meyering wrote:
>> I was thinking about something like:
>> - do ususal things
>> - setup special signal handling
>> - print + process_signals () at print_name_with_quoting
>> - back to original signal handling
>> - do the rest of things
>>
>> so most of the time there won't be any special signal handling (== will be the
>> same as ls without --color).
>>
>>> Do you get the delays with -U too?
>>
>> Yes, too.
>>
>> ls doesn't call process_signals() at all before printing starts in --color
>> mode thus making ls uninterruptible in that period.
> 
> Thanks for the feedback.
> Here's a more complete patch, but I haven't re-reviewed it yet,
> so caveat emptor.

Question - is ls processing entirely two-phase (collect all names, then
format them), or is this a repetitive loop?  Seeing as how long listings
of two directories on the command line produces differing column widths
between the two directories, I have to think it is the latter (besides,
that makes better sense from a memory management perspective - it's
cheaper to sort one directory at a time than to store the sorting of all
directories at once).  In which case, consider:

ls --color=always largedir1 largedir2

If we enable our signal handle just before outputting the sorted,
columnized, and colred largedir1 listings, only to then start our
readdir() of largedir2, we haven't solved the problem - the second
readdir() and/or calculate_columns() between the two directories will be
lengthy enough to be noticeably interrupt-starved.

> @@ -4092,6 +4100,15 @@ static size_t
>  print_file_name_and_frills (const struct fileinfo *f, size_t start_col)
>  {
>    char buf[MAX (LONGEST_HUMAN_READABLE + 1, INT_BUFSIZE_BOUND (uintmax_t))];
> +  static bool first = true;
> +  if (first)
> +    {
> +      /* handle interrupts so that ls cannot be made to output an
> +         incomplete multi-byte sequence or a color-change escape
> +         sequence that could leave the terminal messed up.  */

That is, I think a one-shot static is wrong, and that you will have to
repeatedly install and uninstall the signal handlers, according to which
phase of the processing loop you are in.

-- 
Eric Blake   [email-masked]    +1-919-301-3266
Libvirt virtualization library http://libvirt.org