- csplit sometimes failed to remove files when interrupted.

 - csplit didn't clean up if two signals arrived nearly simultaneously.

(sigprocmask, sigset_t) [!defined SA_NOCLDSTOP]: Define.
(filename_space, prefix, suffix, digits, files_created, remove_files): Now volatile.
(caught_signals): New var.
(cleanup): Block signals while deleting all files.
(cleanup_fatal, handle_line_error, regexp_error):
Mark with ATTRIBUTE_NORETURN.
(create_output_file, close_output_file, interrupt_handler):
Block signals while changing the number of output files,
to fix some race conditions.
(delete_all_files): Do nothing if remove_files is zero.
Clear files_created.
(main): Don't mess with signals until after argument processing is done.

(main): Rewrite signal-catching code to make it
similar to other coreutils programs.  When processing signals,
block all signals that we catch, but do not block signals that we
don't catch.  Avoid problems with unsigned int warnings.

(interrupt_handler): Use void, not (obsolete) RETSIGTYPE.

(interrupt_handler) [defined SA_NOCLDSTOP]:
Use simpler "signal (sig, SIG_DFL)" rather than sigaction equivalent.

1 files changed, 93 insertions, 59 deletions

diff --git a/src/csplit.c b/src/csplit.c
index b0758102e..d1bc0ff8a 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -34,6 +34,11 @@
 #include "safe-read.h"
 #include "xstrtol.h"
 
+#ifndef SA_NOCLDSTOP
+# define sigprocmask(How, Set, Oset) /* empty */
+# define sigset_t int
+#endif
+
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "csplit"
 
@@ -139,19 +144,19 @@ static uintmax_t current_line = 0;
 static bool have_read_eof = false;
 
 /* Name of output files. */
-static char *filename_space = NULL;
+static char * volatile filename_space = NULL;
 
 /* Prefix part of output file names. */
-static char *prefix = NULL;
+static char * volatile prefix = NULL;
 
 /* Suffix part of output file names. */
-static char *suffix = NULL;
+static char * volatile suffix = NULL;
 
 /* Number of digits to use in output file names. */
-static int digits = 2;
+static int volatile digits = 2;
 
 /* Number of files created so far. */
-static unsigned int files_created = 0;
+static unsigned int volatile files_created = 0;
 
 /* Number of bytes written to current file. */
 static uintmax_t bytes_written;
@@ -169,7 +174,7 @@ static char **global_argv;
 static bool suppress_count;
 
 /* If true, remove output files on error. */
-static bool remove_files;
+static bool volatile remove_files;
 
 /* If true, remove all output files which have a zero length. */
 static bool elide_empty_files;
@@ -181,6 +186,9 @@ static struct control *controls;
 /* Number of elements in `controls'. */
 static size_t control_used;
 
+/* The set of signals that are caught.  */
+static sigset_t caught_signals;
+
 static struct option const longopts[] =
 {
   {"digits", required_argument, NULL, 'n'},
@@ -201,12 +209,16 @@ static struct option const longopts[] =
 static void
 cleanup (void)
 {
+  sigset_t oldset;
+
   close_output_file ();
 
-  if (remove_files)
-    delete_all_files ();
+  sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+  delete_all_files ();
+  sigprocmask (SIG_SETMASK, &oldset, NULL);
 }
 
+static void cleanup_fatal (void) ATTRIBUTE_NORETURN;
 static void
 cleanup_fatal (void)
 {
@@ -214,20 +226,16 @@ cleanup_fatal (void)
   exit (EXIT_FAILURE);
 }
 
-static RETSIGTYPE
+static void
 interrupt_handler (int sig)
 {
-#ifdef SA_NOCLDSTOP
-  struct sigaction sigact;
+#ifndef SA_NOCLDSTOP
+  signal (sig, SIG_IGN);
+#endif
+
+  delete_all_files ();
 
-  sigact.sa_handler = SIG_DFL;
-  sigemptyset (&sigact.sa_mask);
-  sigact.sa_flags = 0;
-  sigaction (sig, &sigact, NULL);
-#else
   signal (sig, SIG_DFL);
-#endif
-  cleanup ();
   raise (sig);
 }
 
@@ -681,6 +689,8 @@ dump_rest_of_file (void)
 /* Handle an attempt to read beyond EOF under the control of record P,
    on iteration REPETITION if nonzero. */
 
+static void handle_line_error (const struct control *, uintmax_t)
+     ATTRIBUTE_NORETURN;
 static void
 handle_line_error (const struct control *p, uintmax_t repetition)
 {
@@ -728,6 +738,7 @@ process_line_count (const struct control *p, uintmax_t repetition)
     handle_line_error (p, repetition);
 }
 
+static void regexp_error (struct control *, uintmax_t, bool) ATTRIBUTE_NORETURN;
 static void
 regexp_error (struct control *p, uintmax_t repetition, bool ignore)
 {
@@ -902,30 +913,47 @@ make_filename (unsigned int num)
 static void
 create_output_file (void)
 {
+  sigset_t oldset;
+  bool fopen_ok;
+  int fopen_errno;
+
   output_filename = make_filename (files_created);
+
+  /* Create the output file in a critical section, to avoid races.  */
+  sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
   output_stream = fopen (output_filename, "w");
-  if (output_stream == NULL)
+  fopen_ok = (output_stream != NULL);
+  fopen_errno = errno;
+  files_created += fopen_ok;
+  sigprocmask (SIG_SETMASK, &oldset, NULL);
+
+  if (! fopen_ok)
     {
-      error (0, errno, "%s", output_filename);
+      error (0, fopen_errno, "%s", output_filename);
       cleanup_fatal ();
     }
-  files_created++;
   bytes_written = 0;
 }
 
-/* Delete all the files we have created. */
+/* If requested, delete all the files we have created.  This function
+   must be called only from critical sections.  */
 
 static void
 delete_all_files (void)
 {
   unsigned int i;
 
+  if (! remove_files)
+    return;
+
   for (i = 0; i < files_created; i++)
     {
       const char *name = make_filename (i);
       if (unlink (name))
 	error (0, errno, "%s", name);
     }
+
+  files_created = 0;
 }
 
 /* Close the current output file and print the count
@@ -950,9 +978,19 @@ close_output_file (void)
 	}
       if (bytes_written == 0 && elide_empty_files)
 	{
-	  if (unlink (output_filename))
-	    error (0, errno, "%s", output_filename);
-	  files_created--;
+	  sigset_t oldset;
+	  bool unlink_ok;
+	  int unlink_errno;
+
+	  /* Remove the output file in a critical section, to avoid races.  */
+	  sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+	  unlink_ok = (unlink (output_filename) == 0);
+	  unlink_errno = errno;
+	  files_created -= unlink_ok;
+	  sigprocmask (SIG_SETMASK, &oldset, NULL);
+
+	  if (! unlink_ok)
+	    error (0, unlink_errno, "%s", output_filename);
 	}
       else
 	{
@@ -1273,9 +1311,6 @@ main (int argc, char **argv)
 {
   int optc;
   unsigned long int val;
-#ifdef SA_NOCLDSTOP
-  struct sigaction oldact, newact;
-#endif
 
   initialize_main (&argc, &argv);
   program_name = argv[0];
@@ -1295,37 +1330,6 @@ main (int argc, char **argv)
   /* Change the way xmalloc and xrealloc fail.  */
   xalloc_fail_func = cleanup;
 
-#ifdef SA_NOCLDSTOP
-  newact.sa_handler = interrupt_handler;
-  sigemptyset (&newact.sa_mask);
-  newact.sa_flags = 0;
-
-  sigaction (SIGHUP, NULL, &oldact);
-  if (oldact.sa_handler != SIG_IGN)
-    sigaction (SIGHUP, &newact, NULL);
-
-  sigaction (SIGINT, NULL, &oldact);
-  if (oldact.sa_handler != SIG_IGN)
-    sigaction (SIGINT, &newact, NULL);
-
-  sigaction (SIGQUIT, NULL, &oldact);
-  if (oldact.sa_handler != SIG_IGN)
-    sigaction (SIGQUIT, &newact, NULL);
-
-  sigaction (SIGTERM, NULL, &oldact);
-  if (oldact.sa_handler != SIG_IGN)
-    sigaction (SIGTERM, &newact, NULL);
-#else
-  if (signal (SIGHUP, SIG_IGN) != SIG_IGN)
-    signal (SIGHUP, interrupt_handler);
-  if (signal (SIGINT, SIG_IGN) != SIG_IGN)
-    signal (SIGINT, interrupt_handler);
-  if (signal (SIGQUIT, SIG_IGN) != SIG_IGN)
-    signal (SIGQUIT, interrupt_handler);
-  if (signal (SIGTERM, SIG_IGN) != SIG_IGN)
-    signal (SIGTERM, interrupt_handler);
-#endif
-
   while ((optc = getopt_long (argc, argv, "f:b:kn:sqz", longopts, NULL)) != -1)
     switch (optc)
       {
@@ -1383,6 +1387,36 @@ main (int argc, char **argv)
 
   parse_patterns (argc, optind, argv);
 
+  {
+    int i;
+    static int const sig[] = { SIGHUP, SIGINT, SIGQUIT, SIGTERM };
+    enum { nsigs = sizeof sig / sizeof sig[0] };
+
+#ifdef SA_NOCLDSTOP
+    struct sigaction act;
+
+    sigemptyset (&caught_signals);
+    for (i = 0; i < nsigs; i++)
+      {
+	sigaction (sig[i], NULL, &act);
+	if (act.sa_handler != SIG_IGN)
+	  sigaddset (&caught_signals, sig[i]);
+      }
+
+    act.sa_handler = interrupt_handler;
+    act.sa_mask = caught_signals;
+    act.sa_flags = 0;
+
+    for (i = 0; i < nsigs; i++)
+      if (sigismember (&caught_signals, sig[i]))
+	sigaction (sig[i], &act, NULL);
+#else
+    for (i = 0; i < nsigs; i++)
+      if (signal (sig[i], SIG_IGN) != SIG_IGN)
+	signal (sig[i], interrupt_handler);
+#endif
+  }
+
   split_file ();
 
   if (close (input_desc) < 0)