Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

[Giuseppe Scrivano]

Hello,

do you have suggestions on this patch?  It replaces any `signal' with
`sigaction'.

Regards,
Giuseppe Scrivano

Jim Meyering <address@hidden> writes:

> Good idea.
> I've wanted to get rid of "signal" uses for ages.
> Are you interested in doing it?


diff --git a/src/csplit.c b/src/csplit.c
index 55489c3..565cfbf 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -238,12 +238,22 @@ xalloc_die (void)
 static void
 interrupt_handler (int sig)
 {
+  struct sigaction sa;
+
+  memset (&sa, 0, sizeof sa);
+  sigemptyset (&sa.sa_mask);
+
   if (! SA_NOCLDSTOP)
-    signal (sig, SIG_IGN);
+    {
+      sa.sa_handler = SIG_IGN;
+      sigaction (sig, &sa, NULL);
+    }
 
   delete_all_files (true);
 
-  signal (sig, SIG_DFL);
+  sa.sa_handler = SIG_DFL;
+  sigaction (sig, &sa, NULL);
+
   raise (sig);
 }
 
@@ -1441,11 +1451,19 @@ main (int argc, char **argv)
        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);
-         siginterrupt (sig[i], 1);
-       }
+      {
+        struct sigaction sa;
+        sigaction (sig[i], NULL, &sa);
+
+        if (sa.sa_handler != SIG_IGN)
+          {
+            memset (&sa, 0, sizeof sa);
+            sigemptyset (&sa.sa_mask);
+            sa.sa_handler = interrupt_handler;
+            sa.sa_flags = SA_RESTART;
+            sigaction (sig[i], &sa, NULL);
+          }
+      }
 #endif
   }
 
diff --git a/src/dd.c b/src/dd.c
index f598e44..adea627 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -674,7 +674,15 @@ static void
 interrupt_handler (int sig)
 {
   if (! SA_RESETHAND)
-    signal (sig, SIG_DFL);
+    {
+      struct sigaction sa_default;
+      memset (&sa_default, 0, sizeof sa_default);
+      sigemptyset (&sa_default.sa_mask);
+      sa_default.sa_handler = SIG_DFL;
+
+      sigaction (sig, &sa_default, NULL);
+    }
+
   interrupt_signal = sig;
 }
 
@@ -684,7 +692,14 @@ static void
 siginfo_handler (int sig)
 {
   if (! SA_NOCLDSTOP)
-    signal (sig, siginfo_handler);
+    {
+      struct sigaction sa_default;
+      memset (&sa_default, 0, sizeof sa_default);
+      sigemptyset (&sa_default.sa_mask);
+      sa_default.sa_handler = siginfo_handler;
+
+      sigaction (sig, &sa_default, NULL);
+    }
   info_signal_count++;
 }
 
@@ -729,16 +744,32 @@ install_signal_handlers (void)
 
 #else
 
-  if (catch_siginfo && signal (SIGINFO, SIG_IGN) != SIG_IGN)
-    {
-      signal (SIGINFO, siginfo_handler);
-      siginterrupt (SIGINFO, 1);
-    }
-  if (signal (SIGINT, SIG_IGN) != SIG_IGN)
-    {
-      signal (SIGINT, interrupt_handler);
-      siginterrupt (SIGINT, 1);
-    }
+  {
+    struct sigaction sa;
+    sigaction (SIGINFO, NULL, &sa);
+
+    if (catch_siginfo && sa.sa_handler != SIG_IGN)
+      {
+        memset (&sa, 0, sizeof sa);
+        sigemptyset (&sa.sa_mask);
+        
+        sa.sa_handler = siginfo_handler;
+        sa.sa_flags = SA_RESTART;
+        sigaction (SIGINFO, &sa, NULL);
+      }
+
+    sigaction (SIGINT, NULL, &sa);
+
+    if (catch_siginfo && sa.sa_handler != SIG_IGN)
+      {
+        memset (&sa, 0, sizeof sa);
+        sigemptyset (&sa.sa_mask);
+        
+        sa.sa_handler = interrupt_handler;
+        sa.sa_flags = SA_RESTART;
+        sigaction (SIGINT, &sa, NULL);
+      }
+  }
 #endif
 }
 
diff --git a/src/install.c b/src/install.c
index a7c3b3d..deea27c 100644
--- a/src/install.c
+++ b/src/install.c
@@ -376,7 +376,12 @@ main (int argc, char **argv)
          strip_files = true;
 #ifdef SIGCHLD
          /* System V fork+wait does not work if SIGCHLD is ignored.  */
-         signal (SIGCHLD, SIG_DFL);
+    struct sigaction sa_default;
+    memset (&sa_default, 0, sizeof sa_default);
+    sigemptyset (&sa_default.sa_mask);
+    sa_default.sa_handler = SIG_DFL;
+
+    sigaction (SIGCHLD, &sa_default, NULL);
 #endif
          break;
        case STRIP_PROGRAM_OPTION:
diff --git a/src/ls.c b/src/ls.c
index e107162..905eb7c 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1051,7 +1051,16 @@ static void
 sighandler (int sig)
 {
   if (! SA_NOCLDSTOP)
-    signal (sig, SIG_IGN);
+    {
+      struct sigaction sa_ignore;
+
+      memset (&sa_ignore, 0, sizeof sa_ignore);
+      sigemptyset (&sa_ignore.sa_mask);
+      sa_ignore.sa_handler = SIG_IGN;
+
+      sigaction (sig, &sa_ignore, NULL);
+    }
+
   if (! interrupt_signal)
     interrupt_signal = sig;
 }
@@ -1062,7 +1071,15 @@ static void
 stophandler (int sig)
 {
   if (! SA_NOCLDSTOP)
-    signal (sig, stophandler);
+    {
+      struct sigaction sa_ignore;
+
+      memset (&sa_ignore, 0, sizeof sa_ignore);
+      sigemptyset (&sa_ignore.sa_mask);
+      sa_ignore.sa_handler = SIG_IGN;
+
+      sigaction (sig, &sa_ignore, NULL);
+    }
   if (! interrupt_signal)
     stop_signal_count++;
 }
@@ -1102,7 +1119,14 @@ process_signals (void)
          sig = SIGSTOP;
        }
       else
-       signal (sig, SIG_DFL);
+  {
+    struct sigaction sa_default;
+    memset (&sa_default, 0, sizeof sa_default);
+    sigemptyset (&sa_default.sa_mask);
+    sa_default.sa_handler = SIG_DFL;
+    
+    sigaction (sig, &sa_default, NULL);
+  }
 
       /* Exit or suspend the program.  */
       raise (sig);
@@ -1208,18 +1232,32 @@ main (int argc, char **argv)
          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);
+    struct sigaction sa;
+
+    memset (&sa, 0, sizeof sa);
+    sigemptyset (&sa.sa_mask);
+    sa.sa_handler = sig[j] == SIGTSTP ? stophandler : sighandler;
+
+    sigaction (sig[j], &sa, NULL);
              }
 #else
          for (j = 0; j < nsigs; j++)
            {
-             caught_sig[j] = (signal (sig[j], SIG_IGN) != SIG_IGN);
+        struct sigaction sa;
+        struct sigaction old_sa;
+
+        memset (&sa, 0, sizeof sa);
+        sigemptyset (&sa.sa_mask);
+        sa.sa_handler = SIG_IGN;
+
+        sigaction (sig[j], &sa, &old_sa);
+
+             caught_sig[j] = (old_sa.sa_handler != SIG_IGN);
              if (caught_sig[j])
-               {
-                 signal (sig[j], sig[j] == SIGTSTP ? stophandler : sighandler);
-                 siginterrupt (sig[j], 0);
-               }
+          {
+            sa.sa_handler = sig[j] == SIGTSTP ? stophandler : sighandler;
+            sigaction (sig[j], &sa, NULL);
+          }
            }
 #endif
        }
@@ -1336,6 +1374,10 @@ main (int argc, char **argv)
   if (print_with_color)
     {
       int j;
+      struct sigaction sa_default;
+      memset (&sa_default, 0, sizeof sa_default);
+      sigemptyset (&sa_default.sa_mask);
+      sa_default.sa_handler = SIG_DFL;
 
       if (used_color)
        restore_default_color ();
@@ -1345,11 +1387,11 @@ main (int argc, char **argv)
 #if SA_NOCLDSTOP
       for (j = 0; j < nsigs; j++)
        if (sigismember (&caught_signals, sig[j]))
-         signal (sig[j], SIG_DFL);
+    sigaction (sig[j], &sa_default, NULL);
 #else
       for (j = 0; j < nsigs; j++)
        if (caught_sig[j])
-         signal (sig[j], SIG_DFL);
+    sigaction (sig[j], &sa_default, NULL);
 #endif
 
       /* Act on any signals that arrived before the default was restored.
diff --git a/src/nohup.c b/src/nohup.c
index 3364e91..28d3152 100644
--- a/src/nohup.c
+++ b/src/nohup.c
@@ -85,6 +85,7 @@ main (int argc, char **argv)
   bool redirecting_stdout;
   bool stdout_is_closed;
   bool redirecting_stderr;
+  struct sigaction sa_ignore;
 
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -195,7 +196,11 @@ main (int argc, char **argv)
        close (out_fd);
     }
 
-  signal (SIGHUP, SIG_IGN);
+  memset (&sa_ignore, 0, sizeof sa_ignore);
+  sigemptyset (&sa_ignore.sa_mask);
+  sa_ignore.sa_handler = SIG_IGN;
+  
+  sigaction (SIGHUP, &sa_ignore, NULL); 
 
   {
     int exit_status;
diff --git a/src/sort.c b/src/sort.c
index 43c28fc..578401a 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -2813,12 +2813,22 @@ parse_field_count (char const *string, size_t *val, 
char const *msgid)
 static void
 sighandler (int sig)
 {
+  struct sigaction sa;
+
+  memset (&sa, 0, sizeof sa);
+  sigemptyset (&sa.sa_mask);
+
   if (! SA_NOCLDSTOP)
-    signal (sig, SIG_IGN);
+    {
+      sa.sa_handler = SIG_IGN;
+      sigaction (sig, &sa, NULL);
+    }
 
   cleanup ();
 
-  signal (sig, SIG_DFL);
+  sa.sa_handler = SIG_DFL;
+  sigaction (sig, &sa, NULL);
+
   raise (sig);
 }
 
@@ -2983,11 +2993,19 @@ main (int argc, char **argv)
        sigaction (sig[i], &act, NULL);
 #else
     for (i = 0; i < nsigs; i++)
-      if (signal (sig[i], SIG_IGN) != SIG_IGN)
-       {
-         signal (sig[i], sighandler);
-         siginterrupt (sig[i], 1);
-       }
+      {
+        struct sigaction sa;
+        sigaction (sig[i], NULL, &sa);
+
+        if (sa.sa_handler != SIG_IGN)
+          {
+            memset (&sa, 0, sizeof sa);
+            sigemptyset (&sa.sa_mask);
+            sa.sa_handler = sighandler;
+            sa.sa_flags = SA_RESTART;
+            sigaction (sig[i], &sa, NULL);
+          }
+      }
 #endif
   }
 
diff --git a/src/tee.c b/src/tee.c
index 4e46aab..580796c 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -115,7 +115,14 @@ main (int argc, char **argv)
     }
 
   if (ignore_interrupts)
-    signal (SIGINT, SIG_IGN);
+  {
+    struct sigaction sa_ignore;
+    memset (&sa_ignore, 0, sizeof sa_ignore);
+    sigemptyset (&sa_ignore.sa_mask);
+    sa_ignore.sa_handler = SIG_IGN;
+
+    sigaction (SIGINT, &sa_ignore, NULL);
+  }
 
   /* Do *not* warn if tee is given no file arguments.
      POSIX requires that it work when given no arguments.  */
diff --git a/src/timeout.c b/src/timeout.c
index 8b506f0..6ef7218 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -278,8 +278,6 @@ main (int argc, char **argv)
   /* Setup handlers before fork() so that we
      handle any signals caused by child, without races.  */
   install_signal_handlers ();
-  signal (SIGTTIN, SIG_IGN);    /* don't sTop if background child needs tty.  
*/
-  signal (SIGTTOU, SIG_IGN);    /* don't sTop if background child needs tty.  
*/
 
   monitored_pid = fork ();
   if (monitored_pid == -1)
@@ -291,10 +289,6 @@ main (int argc, char **argv)
     {                           /* child */
       int exit_status;
 
-      /* exec doesn't reset SIG_IGN -> SIG_DFL.  */
-      signal (SIGTTIN, SIG_DFL);
-      signal (SIGTTOU, SIG_DFL);
-
       execvp (argv[0], argv);   /* FIXME: should we use "sh -c" ... here?  */
 
       /* exit like sh, env, nohup, ...  */
@@ -305,6 +299,15 @@ main (int argc, char **argv)
   else
     {
       int status;
+      struct sigaction sa_ignore;
+
+      memset (&sa_ignore, 0, sizeof sa_ignore);
+      sigemptyset (&sa_ignore.sa_mask);
+      sa_ignore.sa_handler = SIG_IGN;
+
+      sigaction (SIGTTIN, &sa_ignore, NULL);     /* don't sTop if background 
child needs tty.  */
+      sigaction (SIGTTOU, &sa_ignore, NULL);    /* don't sTop if background 
child needs tty.  */
+
 
       alarm ((unsigned int) timeout);