bug#24232: [PATCH] ls: postpone installation of signal handlers

[Kamil Dudka]

... until they are actually needed.  That is right before the first
escape sequence is printed to a terminal.

* src/ls.c (sigs): Moved from main() to global scope to make it
available in put_indicator(), too.  Renamed to prevent identifier
clashes with parameters of subsequent function definitions.
(nsigs): Likewise.
(caught_sig): Likewise.
(main): Code installing signal handlers moved to put_indicator() in
order not to keep default signal handling untouched as long as possible.
Adjusted condition for restoring signal handlers to reflect the change.
(put_indicator): Install signal handlers if called for the very first
time.  It uses the same code that was in main() prior to this commit.
* NEWS: Mention the change.

See https://bugzilla.redhat.com/1365933
---
 NEWS     |   3 ++
 src/ls.c | 167 +++++++++++++++++++++++++++++++--------------------------------
 2 files changed, 86 insertions(+), 84 deletions(-)

diff --git a/NEWS b/NEWS
index 736b95e..d4af96f 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   System V style platforms where this information is available only
   in the global variable 'tzname'. [bug introduced in coreutils-8.24]
 
+  ls is now fully responsive to signals until the first escape sequence is
+  written to a terminal.
+
   nl now resets numbering for each page section rather than just for each page.
   [This bug was present in "the beginning".]
 
diff --git a/src/ls.c b/src/ls.c
index 3572060..c438f61 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -753,6 +753,36 @@ static char const *long_time_format[2] =
     N_("%b %e %H:%M")
   };
 
+/* The signals that are trapped, and the number of such signals.  */
+static int const sigs[] =
+{
+  /* This one is handled specially.  */
+  SIGTSTP,
+
+  /* The usual suspects.  */
+  SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
+#ifdef SIGPOLL
+  SIGPOLL,
+#endif
+#ifdef SIGPROF
+  SIGPROF,
+#endif
+#ifdef SIGVTALRM
+  SIGVTALRM,
+#endif
+#ifdef SIGXCPU
+  SIGXCPU,
+#endif
+#ifdef SIGXFSZ
+  SIGXFSZ,
+#endif
+};
+enum { nsigs = ARRAY_CARDINALITY (sigs) };
+
+#if ! SA_NOCLDSTOP
+static bool caught_sig[nsigs];
+#endif
+
 /* The set of signals that are caught.  */
 
 static sigset_t caught_signals;
@@ -1251,36 +1281,6 @@ main (int argc, char **argv)
   struct pending *thispend;
   int n_files;
 
-  /* The signals that are trapped, and the number of such signals.  */
-  static int const sig[] =
-    {
-      /* This one is handled specially.  */
-      SIGTSTP,
-
-      /* The usual suspects.  */
-      SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
-#ifdef SIGPOLL
-      SIGPOLL,
-#endif
-#ifdef SIGPROF
-      SIGPROF,
-#endif
-#ifdef SIGVTALRM
-      SIGVTALRM,
-#endif
-#ifdef SIGXCPU
-      SIGXCPU,
-#endif
-#ifdef SIGXFSZ
-      SIGXFSZ,
-#endif
-    };
-  enum { nsigs = ARRAY_CARDINALITY (sig) };
-
-#if ! SA_NOCLDSTOP
-  bool caught_sig[nsigs];
-#endif
-
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
   setlocale (LC_ALL, "");
@@ -1314,46 +1314,6 @@ main (int argc, char **argv)
           || (is_colored (C_EXEC) && color_symlink_as_referent)
           || (is_colored (C_MISSING) && format == long_format))
         check_symlink_color = true;
-
-      /* If the standard output is a controlling terminal, watch out
-         for signals, so that the colors can be restored to the
-         default state if "ls" is suspended or interrupted.  */
-
-      if (0 <= tcgetpgrp (STDOUT_FILENO))
-        {
-          int j;
-#if SA_NOCLDSTOP
-          struct sigaction act;
-
-          sigemptyset (&caught_signals);
-          for (j = 0; j < nsigs; j++)
-            {
-              sigaction (sig[j], NULL, &act);
-              if (act.sa_handler != SIG_IGN)
-                sigaddset (&caught_signals, sig[j]);
-            }
-
-          act.sa_mask = caught_signals;
-          act.sa_flags = SA_RESTART;
-
-          for (j = 0; j < nsigs; j++)
-            if (sigismember (&caught_signals, sig[j]))
-              {
-                act.sa_handler = sig[j] == SIGTSTP ? stophandler : sighandler;
-                sigaction (sig[j], &act, NULL);
-              }
-#else
-          for (j = 0; j < nsigs; j++)
-            {
-              caught_sig[j] = (signal (sig[j], SIG_IGN) != SIG_IGN);
-              if (caught_sig[j])
-                {
-                  signal (sig[j], sig[j] == SIGTSTP ? stophandler : 
sighandler);
-                  siginterrupt (sig[j], 0);
-                }
-            }
-#endif
-        }
     }
 
   if (dereference == DEREF_UNDEFINED)
@@ -1466,31 +1426,29 @@ main (int argc, char **argv)
       print_dir_name = true;
     }
 
-  if (print_with_color)
+  if (print_with_color && used_color)
     {
       int j;
 
-      if (used_color)
-        {
-          /* Skip the restore when it would be a no-op, i.e.,
-             when left is "\033[" and right is "m".  */
-          if (!(color_indicator[C_LEFT].len == 2
-                && memcmp (color_indicator[C_LEFT].string, "\033[", 2) == 0
-                && color_indicator[C_RIGHT].len == 1
-                && color_indicator[C_RIGHT].string[0] == 'm'))
-            restore_default_color ();
-        }
+      /* Skip the restore when it would be a no-op, i.e.,
+         when left is "\033[" and right is "m".  */
+      if (!(color_indicator[C_LEFT].len == 2
+            && memcmp (color_indicator[C_LEFT].string, "\033[", 2) == 0
+            && color_indicator[C_RIGHT].len == 1
+            && color_indicator[C_RIGHT].string[0] == 'm'))
+        restore_default_color ();
+
       fflush (stdout);
 
       /* Restore the default signal handling.  */
 #if SA_NOCLDSTOP
       for (j = 0; j < nsigs; j++)
-        if (sigismember (&caught_signals, sig[j]))
-          signal (sig[j], SIG_DFL);
+        if (sigismember (&caught_signals, sigs[j]))
+          signal (sigs[j], SIG_DFL);
 #else
       for (j = 0; j < nsigs; j++)
         if (caught_sig[j])
-          signal (sig[j], SIG_DFL);
+          signal (sigs[j], SIG_DFL);
 #endif
 
       /* Act on any signals that arrived before the default was restored.
@@ -4475,6 +4433,47 @@ put_indicator (const struct bin_str *ind)
   if (! used_color)
     {
       used_color = true;
+
+      /* If the standard output is a controlling terminal, watch out
+         for signals, so that the colors can be restored to the
+         default state if "ls" is suspended or interrupted.  */
+
+      if (0 <= tcgetpgrp (STDOUT_FILENO))
+        {
+          int j;
+#if SA_NOCLDSTOP
+          struct sigaction act;
+
+          sigemptyset (&caught_signals);
+          for (j = 0; j < nsigs; j++)
+            {
+              sigaction (sigs[j], NULL, &act);
+              if (act.sa_handler != SIG_IGN)
+                sigaddset (&caught_signals, sigs[j]);
+            }
+
+          act.sa_mask = caught_signals;
+          act.sa_flags = SA_RESTART;
+
+          for (j = 0; j < nsigs; j++)
+            if (sigismember (&caught_signals, sigs[j]))
+              {
+                act.sa_handler = sigs[j] == SIGTSTP ? stophandler : sighandler;
+                sigaction (sigs[j], &act, NULL);
+              }
+#else
+          for (j = 0; j < nsigs; j++)
+            {
+              caught_sig[j] = (signal (sigs[j], SIG_IGN) != SIG_IGN);
+              if (caught_sig[j])
+                {
+                  signal (sigs[j], sigs[j] == SIGTSTP ? stophandler : 
sighandler);
+                  siginterrupt (sigs[j], 0);
+                }
+            }
+#endif
+        }
+
       prep_non_filename_text ();
     }
 
-- 
2.7.4