Re: bug#9076: coreutils-8.12 uses SA_RESETHAND and SA_RESTART unconditionally

[Paul Eggert]

On 07/14/11 17:25, Pádraig Brady wrote:
> I'm not sure about defining these to 0 in gnulib.
> That will silently ignore the intent of a program on certain platforms.
> Wouldn't it be better to fail to compile so that each program then does:
> 
> #ifndef SA_RESETHAND
> /* do something else */
> #endif

Well, as a matter of style, I prefer this:

   if (! SA_RESETHAND)
     do_something_else ();

as that's much more likely to detect bitrot in the "do something else"
part.  (Of course this is all assuming the application writer knows
about platforms where SA_RESETHAND isn't implemented.)

On NonStop, if you invoke signal(), it uses the SA_RESETHAND semantics
(POSIX allows this).  Conversely, if you invoke sigaction(), NonStop
always behaves as if SA_RESTART and SA_RESETHAND are zero, i.e., it
doesn't support either feature with sigaction.

I just now checked coreutils.  Two programs use SA_RESETHAND.  The
first (csplit) is clearly using it incorrectly, and won't work
properly on any host that implements SA_RESETHAND according to POSIX.
The second (dd) is using the above idiom already, that is, it is
assuming SA_RESETHAND is #defined to 0 on hosts that don't support it.
(It has some obsolete comments though, that need fixing.)

Conversely, the two programs that depend on SA_RESTART will
probably misbehave if a signal interrupts a system call on
NonStop.  I see no easy fix for this, and I expect the gnulib
change may be the best we can do.

Here are a couple of patches that address the SA_RESETHAND problems
mentioned above.  They assume the gnulib change.  They fix the csplit
bug (and also allow it to work on NonStop, assuming the gnulib
change).

diff --git a/src/csplit.c b/src/csplit.c
index 438d888..4a0914e 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -225,6 +225,9 @@ static void
 interrupt_handler (int sig)
 {
   delete_all_files (true);
+
+  signal (sig, SIG_DFL);
+
   /* The signal has been reset to SIG_DFL, but blocked during this
      handler.  Force the default action of this signal once the
      handler returns and the block is removed.  */
@@ -1421,7 +1424,7 @@ main (int argc, char **argv)
 
     act.sa_handler = interrupt_handler;
     act.sa_mask = caught_signals;
-    act.sa_flags = SA_NODEFER | SA_RESETHAND;
+    act.sa_flags = 0;
 
     for (i = 0; i < nsigs; i++)
       if (sigismember (&caught_signals, sig[i]))


diff --git a/src/dd.c b/src/dd.c
index 45aa9b9..c30c78a 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -45,7 +45,7 @@
   proper_name ("Stuart Kemp")
 
 /* Use SA_NOCLDSTOP as a proxy for whether the sigaction machinery is
-   present.  SA_NODEFER and SA_RESETHAND are XSI extensions.  */
+   present.  */
 #ifndef SA_NOCLDSTOP
 # define SA_NOCLDSTOP 0
 # define sigprocmask(How, Set, Oset) /* empty */
@@ -726,9 +726,6 @@ install_signal_handlers (void)
 
   if (sigismember (&caught_signals, SIGINT))
     {
-      /* POSIX 1003.1-2001 says SA_RESETHAND implies SA_NODEFER,
-         but this is not true on Solaris 8 at least.  It doesn't
-         hurt to use SA_NODEFER here, so leave it in.  */
       act.sa_handler = interrupt_handler;
       act.sa_flags = SA_NODEFER | SA_RESETHAND;
       sigaction (SIGINT, &act, NULL);