bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: feature request: gzip/bzip support for sort


From: James Youngman
Subject: Re: feature request: gzip/bzip support for sort
Date: Sun, 21 Jan 2007 15:08:28 +0000

It might be worth ensuring that we don't pass an invalid signal mask
to sigprocmask(SET_MASK,...) if the previous call to
sigprocmask(SIG_BLOCK,...) had failed.  Offhand I can't think of a way
for sigprocmask() to fail unless the first argument is invalid, but
looking that the unchecked return code makes me mildly nervous.  How
about something resembling this (untested since my version of Automake
is still only 1.9.6)?

2007-01-21  James Youngman  <address@hidden>

        Centralize all the uses of sigprocmask().  Don't restore an invalid
        saved mask.
        * src/sort.c (enter_cs, leave_cs): New functions for protecting
       code sequences against signal delivery.
        * (exit_cleanup): Use enter_cs and leave_cs instead of
       calling sigprocmask directly.
        (create_temp_file, pipe_fork, zaptemp): Likewise

--- sort.c      2007-01-21 14:37:20.000000000 +0000
+++ sort_prime.c        2007-01-21 14:37:44.000000000 +0000
@@ -64,7 +64,8 @@ struct rlimit { size_t rlim_cur; };
   present.  */
#ifndef SA_NOCLDSTOP
# define SA_NOCLDSTOP 0
-# define sigprocmask(How, Set, Oset) /* empty */
+/* No sigprocmask.  Always 'return' zero. */
+# define sigprocmask(How, Set, Oset) (0)
# define sigset_t int
# if ! HAVE_SIGINTERRUPT
#  define siginterrupt(sig, flag) /* empty */
@@ -403,6 +404,35 @@ static struct option const long_options[
/* The set of signals that are caught.  */
static sigset_t caught_signals;

+
+struct crit_sec_status
+{
+  int      valid;
+  sigset_t sigs;
+};
+
+
+/* Enter a critical section */
+static struct crit_sec_status
+enter_cs(void)
+{
+  struct crit_sec_status status;
+  status.valid = (0 == sigprocmask (SIG_BLOCK, &caught_signals, &status.sigs));
+  return status;
+}
+
+/* Leave a critical section */
+static int
+leave_cs(struct crit_sec_status status)
+{
+  if (status.valid)
+    {
+      /* Ignore failure when restoring the signal mask. */
+      (void) sigprocmask (SIG_SETMASK, &status.sigs, NULL);
+    }
+}
+
+
/* The list of temporary files. */
struct tempnode
{
@@ -568,10 +598,9 @@ exit_cleanup (void)
    {
      /* Clean up any remaining temporary files in a critical section so
         that a signal handler does not try to clean them too.  */
-      sigset_t oldset;
-      sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+      struct crit_sec_status cs = enter_cs();
      cleanup ();
-      sigprocmask (SIG_SETMASK, &oldset, NULL);
+      leave_cs(cs);
    }

  close_stdout ();
@@ -593,6 +622,7 @@ create_temp_file (int *pfd)
  struct tempnode *node =
    xmalloc (offsetof (struct tempnode, name) + len + sizeof slashbase);
  char *file = node->name;
+  struct crit_sec_status cs;

  memcpy (file, temp_dir, len);
  memcpy (file + len, slashbase, sizeof slashbase);
@@ -602,7 +632,7 @@ create_temp_file (int *pfd)
    temp_dir_index = 0;

  /* Create the temporary file in a critical section, to avoid races.  */
-  sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+  cs = enter_cs();
  fd = mkstemp (file);
  if (0 <= fd)
    {
@@ -610,7 +640,7 @@ create_temp_file (int *pfd)
      temptail = &node->next;
    }
  saved_errno = errno;
-  sigprocmask (SIG_SETMASK, &oldset, NULL);
+  leave_cs(cs);
  errno = saved_errno;

  if (fd < 0)
@@ -694,7 +724,8 @@ pipe_fork (int pipefds[2], size_t tries)
  int saved_errno;
  unsigned int wait_retry = 1;
  pid_t pid;
-
+  struct crit_sec_status cs;
+
  if (pipe (pipefds) < 0)
    return -1;

@@ -702,7 +733,7 @@ pipe_fork (int pipefds[2], size_t tries)
    {
      /* This is so the child process won't delete our temp files
         if it receives a signal before exec-ing.  */
-      sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+      cs = enter_cs();
      saved_temphead = temphead;
      temphead = NULL;

@@ -711,7 +742,7 @@ pipe_fork (int pipefds[2], size_t tries)
      if (pid)
        temphead = saved_temphead;

-      sigprocmask (SIG_SETMASK, &oldset, NULL);
+      leave_cs(cs);
      errno = saved_errno;

      if (0 <= pid || errno != EAGAIN)
@@ -870,17 +901,18 @@ zaptemp (const char *name)
  sigset_t oldset;
  int unlink_status;
  int unlink_errno = 0;
+  struct crit_sec_status cs;

  for (pnode = &temphead; (node = *pnode)->name != name; pnode = &node->next)
    continue;

  /* Unlink the temporary file in a critical section to avoid races.  */
  next = node->next;
-  sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+  cs = enter_cs();
  unlink_status = unlink (name);
  unlink_errno = errno;
  *pnode = next;
-  sigprocmask (SIG_SETMASK, &oldset, NULL);
+  leave_cs(cs);

  if (unlink_status != 0)
    error (0, unlink_errno, _("warning: cannot remove: %s"), name);




reply via email to

[Prev in Thread] Current Thread [Next in Thread]