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

[Jim Meyering]

Eric Blake wrote:
> On 12/07/2011 03:04 PM, Jim Meyering wrote:
...
>> 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

Thanks, you're right.
This is my first use of the new Co-authored-by: syntax,
so I confirmed that our updated gitlog-to-changelog does produce
the expected ChangeLog in the distribution tarball.

Here's a simpler patch.  I've tested it by running

    ls -R --color big something-distinctive big2

where big has 500K entries and big2 has 2M (on a tmpfs).
Before (using my initial patch), I could interrupt any time
before the ls' readdir loop for big2, but not during that loop,
which would run for a few seconds after ls printed its "/t/big2:"
Now, it's interruptible, as one would expect.

>From aaf5b61e9999d0ece522faa4508a6565d51a8446 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 8 Dec 2011 10:49:03 +0100
Subject: [PATCH] ls: be responsive to interrupts when color-listing large
 directories
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Starting with commit adc30a83, when using --color, ls inhibited
interrupts to avoid corrupting the state of an output terminal.
However, for very large directories, that inhibition rendered ls
uninterruptible for too long, including a potentially long period
even before any output is generated.
* src/ls.c: There are two phases of processing that are time-
consuming enough that they can cause this sort of trouble:
the readdir loop and the printing loop.  The printing side of things
was nominally covered by a call to process_signals in
(print_name_with_quoting): ... but that call was mistakenly guarded
by a condition that might be false for many or even all files being
processed.  Call process_signals unconditionally.
(print_dir): Also call process_signals in the readdir loop.
* NEWS (Bug fixes): Mention it.
Reported by Arkadiusz Miśkiewicz in http://bugs.gnu.org/10243
Co-authored-by: Eric Blake <address@hidden>
---
 NEWS      |    3 +++
 THANKS.in |    1 +
 src/ls.c  |    7 ++++++-
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/NEWS b/NEWS
index de3888d..0d4c83b 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 ** Bug fixes

+  ls --color many-entry-directory was uninterruptible for too long
+  [bug introduced in coreutils-5.2.1]
+
   ls's -k option no longer affects how ls -l outputs file sizes.
   It now affects only the per-directory block counts written by -l,
   and the sizes written by -s.  This is for compatibility with BSD
diff --git a/THANKS.in b/THANKS.in
index 5ecc29e..afed5d4 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -59,6 +59,7 @@ Anthony Thyssen                     address@hidden
 Antonio Rendas                      address@hidden
 Ariel Faigon                        address@hidden
 Arjan Opmeer                        address@hidden
+Arkadiusz Miśkiewicz                address@hidden
 Arne Henrik Juul                    address@hidden
 Arnold Robbins                      address@hidden
 Arthur Pool                         address@hidden
diff --git a/src/ls.c b/src/ls.c
index 8be9b6a..0d64bab 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2595,6 +2595,11 @@ print_dir (char const *name, char const *realname, bool 
command_line_arg)
         }
       else
         break;
+
+      /* When processing a very large directory, and since we've inhibited
+         interrupts, this loop would take so long that ls would be annoyingly
+         uninterruptible.  This ensures that it handles signals promptly.  */
+      process_signals ();
     }

   if (closedir (dirp) != 0)
@@ -4060,9 +4065,9 @@ print_name_with_quoting (const struct fileinfo *f,
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);

+  process_signals ();
   if (used_color_this_time)
     {
-      process_signals ();
       prep_non_filename_text ();
       if (start_col / line_length != (start_col + width - 1) / line_length)
         put_indicator (&color_indicator[C_CLR_TO_EOL]);
--
1.7.8.110.g4cb5d1