Subject: bug#34488: Add sort --limit, or document workarounds for sort|head error messages

From: Pádraig Brady <[email-masked]>
Date: Mon, 25 Feb 2019 22:35:19 -0800

Thanks for doing all that.
I've attached a few changes:

- spelling fixes
- usage() clarified/reordered
- ensure sigset_t are initialized
- Don't setprocmask() unless specified
- Simplified SETMASK_SIGNAL_OPTION handling
- The test missed `env` as a prerequisite
- The test was slow/spun cpu, so used sleep instead of seq
- Used $SHELL in case sh didn't support trap

I see that the last signal that `kill -l` lists is not included.
I think we should be processing SIGNUM_BOUND also?

cheers,
Pádraig
>From f54e67f2a9dcc4db287c31969e99899582f53a88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Mon, 25 Feb 2019 22:30:09 -0800
Subject: [PATCH] env: misc signal handling fixes

spelling fixes
usage() clarified/reordered
ensure sigset_t are initialized
Don't setprocmask() unless specified
Simplified SETMASK_SIGNAL_OPTION handling
test missed `env` as a prerequisite
test was slow/spun cpu, so used sleep instead of seq
Used $SHELL in case sh didn't support trap
---
 NEWS                             |  4 ++--
 doc/coreutils.texi               |  6 +++---
 man/env.x                        |  4 ++--
 src/env.c                        | 46 +++++++++++++++++++++++-----------------
 tests/misc/env-signal-handler.sh | 33 ++++++++++++----------------
 5 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/NEWS b/NEWS
index c310d1f..e090c72 100644
--- a/NEWS
+++ b/NEWS
@@ -67,10 +67,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   test now supports the '-N FILE' unary operator (like e.g. bash) to check
   whether FILE exists and has been modified since it was last read.
 
-  env now supports '--default-singal[=SIG]' and '--ignore-signal[=SIG]'
+  env now supports '--default-signal[=SIG]' and '--ignore-signal[=SIG]'
   options to set signal handlers before executing a program.
 
-  env now supports '--{block,unblock,setmask}-singal[=SIG]' to block/unblock
+  env now supports '--{block,unblock,setmask}-signal[=SIG]' to block/unblock
   signal delivery before executing a program.
 
   env now supports '--list-signal-actions' and '--list-blocked-signals'
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 5199b83..30a5990 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17274,7 +17274,7 @@ Most operating systems do not allow ignoring @samp{SIGKILL}, @samp{SIGSTOP}
 
 Multiple (and contradictory) @option{--default-signal=SIG} and
 @option{--ignore-signal=SIG} options are processed left-to-right,
-with the latter taking precedence. In the follwing example, @samp{SIGPIPE} is
+with the latter taking precedence. In the following example, @samp{SIGPIPE} is
 set to default while @samp{SIGINT} is ignored:
 
 @example
@@ -17282,10 +17282,10 @@ env --default-signal=INT,PIPE --ignore-signal=INT
 @end example
 
 @item --block-signal[=@var{sig}]
-Block signal @var{sig} from being delivered.
+Block signal(s) @var{sig} from being delivered.
 
 @item --unblock-signal[=@var{sig}]
-Unblock signal @var{sig}.
+Unblock signal(s) @var{sig}.
 
 @item --setmask-signal[=@var{sig}]
 Set list of blocked signals to @var{sig}. All other signals will be unblocked.
diff --git a/man/env.x b/man/env.x
index b787fe3..b2eb371 100644
--- a/man/env.x
+++ b/man/env.x
@@ -38,7 +38,7 @@ parameter the script will likely fail with:
 .PP
 See the full documentation for more details.
 .PP
-.SS "\-\-default-signal[=SIG]" to 'untrap' a singal
+.SS "\-\-default-signal[=SIG]" to 'untrap' a signal
 This option allows setting a signal handler to its default
 action. This is useful to reset a signal after setting it
 to 'ignore' using the shell's trap command.
@@ -87,7 +87,7 @@ Multiple (and contradictory)
 and
 .B \-\-ignore\-signal=SIG
 options are processed left-to-right, with the latter taking precedence.
-In the follwing example, SIGPIPE is set to default while SIGINT is ignored:
+In the following example, SIGPIPE is set to default while SIGINT is ignored:
 .RS
 .nf
 env \-\-default\-signal=INT,PIPE \-\-ignore\-signal=INT
diff --git a/src/env.c b/src/env.c
index 4385620..1acfc11 100644
--- a/src/env.c
+++ b/src/env.c
@@ -67,6 +67,8 @@ static sigset_t block_signals;
 /* Set of signals to unblock.  */
 static sigset_t unblock_signals;
 
+/* Whether signal mask adjustment requested.  */
+static bool sig_mask_changed;
 
 static char const shortopts[] = "+C:ipS:u:v0 \t";
 
@@ -125,35 +127,32 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   -u, --unset=NAME     remove variable from the environment\n\
 "), stdout);
       fputs (_("\
-      --block-signal[=SIG]    block signal SIG.\n\
+  -C, --chdir=DIR      change working directory to DIR\n\
 "), stdout);
       fputs (_("\
-      --unblock-signal[=SIG]  unblock signal SIG.\n\
+  -S, --split-string=S  process and split S into separate arguments;\n\
+                        used to pass multiple arguments on shebang lines\n\
 "), stdout);
       fputs (_("\
-      --setmask-signal[=SIG]  set blocked signal(s) mask to SIG.\n\
+      --block-signal[=SIG]    block delivery of SIG signal(s) to COMMAND\n\
 "), stdout);
       fputs (_("\
-  -C, --chdir=DIR      change working directory to DIR\n\
+      --unblock-signal[=SIG]  unblock delivery of SIG signal(s) to COMMAND\n\
 "), stdout);
       fputs (_("\
-      --default-signal[=SIG]  set signal SIG to its default action.\n\
+      --setmask-signal[=SIG]  set full mask with SIG signal(s) blocked\n\
 "), stdout);
       fputs (_("\
-      --ignore-signal[=SIG]   set signal SIG to be ignored.\n\
+      --default-signal[=SIG]  reset handling of SIG signal(s) to the default\n\
 "), stdout);
       fputs (_("\
-  -S, --split-string=S  process and split S into separate arguments;\n\
-                        used to pass multiple arguments on shebang lines\n\
+      --ignore-signal[=SIG]   set handling of SIG signals(s) to do nothing\n\
 "), stdout);
       fputs (_("\
-      --list-signal-actions   list non-default actions for signals and exit.\n\
-                              if all signals use their default actions,\n\
-                              nothing is printed.\n\
+      --list-signal-actions   list signals with non-default actions and exit\n\
 "), stdout);
       fputs (_("\
-      --list-blocked-signals  list blocked (masked) signals and exit.\n\
-                              if no signals are blocked, nothing is printed.\n\
+      --list-blocked-signals  list blocked (masked) signals and exit\n\
 "), stdout);
       fputs (_("\
   -p                   same as --default-signal=PIPE\n\
@@ -172,7 +171,6 @@ A mere - implies -i.  If no COMMAND, print the resulting environment.\n\
 SIG may be a signal name like 'PIPE', or a signal number like '13'.\n\
 Without SIG, all known signals are included. Multiple signals can be\n\
 comma-separated.\n\
-\n\
 "), stdout);
       emit_ancillary_info (PROGRAM_NAME);
     }
@@ -723,8 +721,18 @@ parse_block_signal_params (const char* optarg, bool block)
       /* without an argument, reset all signals.  */
       sigfillset (block ? &block_signals : &unblock_signals);
       sigemptyset (block ? &unblock_signals : &block_signals);
-      return;
     }
+  else if (! sig_mask_changed)
+    {
+      /* Initialize the sets.  */
+      sigemptyset (&block_signals);
+      sigemptyset (&unblock_signals);
+    }
+
+  sig_mask_changed = true;
+
+  if (! optarg)
+    return;
 
   optarg_writable = xstrdup (optarg);
 
@@ -780,7 +788,7 @@ set_signal_proc_mask (void)
         {
           char signame[SIG2STR_MAX];
           sig2str (i, signame);
-          devmsg ("signal %s (%d) procmask set to %s\n",
+          devmsg ("signal %s (%d) mask set to %s\n",
                   signame, i, debug_act);
         }
     }
@@ -865,8 +873,7 @@ main (int argc, char **argv)
           parse_block_signal_params (optarg, false);
           break;
         case SETMASK_SIGNAL_OPTION:
-          sigfillset (&unblock_signals);
-          sigemptyset (&block_signals);
+          parse_block_signal_params (NULL, false);
           parse_block_signal_params (optarg, true);
           break;
         case LIST_BLOCKED_SIGNALS_OPTION:
@@ -948,7 +955,8 @@ main (int argc, char **argv)
     }
 
   reset_signal_handlers ();
-  set_signal_proc_mask ();
+  if (sig_mask_changed)
+    set_signal_proc_mask ();
 
   if (newdir)
     {
diff --git a/tests/misc/env-signal-handler.sh b/tests/misc/env-signal-handler.sh
index b531549..4dd90c2 100755
--- a/tests/misc/env-signal-handler.sh
+++ b/tests/misc/env-signal-handler.sh
@@ -17,9 +17,7 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
-print_ver_ seq
-print_ver_ timeout
-print_ver_ test
+print_ver_ env seq test timeout
 trap_sigpipe_or_skip_
 
 # Paraphrasing http://bugs.gnu.org/34488#8:
@@ -40,7 +38,7 @@ trap_sigpipe_or_skip_
 # sets SIGPIPE to ignore, and the second 'trap' becomes a no-op instead
 # of resetting SIGPIPE to its default). Upon a SIGPIPE 'seq' will not be
 # terminated, instead its write(2) call will return an error.
-(trap '' PIPE; sh -c 'trap - PIPE; seq 999999 2>err1t | head -n1 > out1')
+(trap '' PIPE; $SHELL -c 'trap - PIPE; seq 999999 2>err1t | head -n1 > out1')
 
 # The exact broken pipe message depends on the operating system, just ensure
 # there was a 'write error' message in stderr:
@@ -60,7 +58,7 @@ compare exp-err1 err1 || framework_failure_
 # 'seq' program):
 (trap '' PIPE;
  env --default-signal=PIPE \
-    sh -c 'trap - PIPE; seq 999999 2>err2 | head -n1 > out2')
+    $SHELL -c 'trap - PIPE; seq 999999 2>err2 | head -n1 > out2')
 
 compare exp-out out2 || fail=1
 compare /dev/null err2 || fail=1
@@ -70,7 +68,7 @@ compare /dev/null err2 || fail=1
 # Repeat the previous test, using "-p" (shortcut for --default-signal=PIPE):
 (trap '' PIPE;
  env -p \
-    sh -c 'trap - PIPE; seq 999999 2>err3 | head -n1 > out3')
+    $SHELL -c 'trap - PIPE; seq 999999 2>err3 | head -n1 > out3')
 
 compare exp-out out3 || fail=1
 compare /dev/null err3 || fail=1
@@ -81,25 +79,21 @@ compare /dev/null err3 || fail=1
 # i.e., all signals.
 (trap '' PIPE;
  env --default-signal \
-    sh -c 'trap - PIPE; seq 999999 2>err4 | head -n1 > out4')
+    $SHELL -c 'trap - PIPE; seq 999999 2>err4 | head -n1 > out4')
 
 compare exp-out out4 || fail=1
 compare /dev/null err4 || fail=1

-
-
-
-
 # Baseline test - ignore signal handler
 # -------------------------------------
-# Kill 'seq' after 1 second with SIGINT - it should terminate (as SIGINT's
+# Kill 'sleep' after 1 second with SIGINT - it should terminate (as SIGINT's
 # default action is to terminate a program).
 # (The first 'env' is just to ensure timeout is not the shell's built-in.)
-env timeout --verbose --kill-after=1 --signal=INT 1 \
-    seq inf > /dev/null 2>err5
+env timeout --verbose --kill-after=.1 --signal=INT .1 \
+    sleep 10 > /dev/null 2>err5
 
-printf "timeout: sending signal INT to command 'seq'\n" > exp-err5 \
+printf "timeout: sending signal INT to command 'sleep'\n" > exp-err5 \
     || framework_failure_
 
 compare exp-err5 err5 || fail=1
@@ -115,9 +109,9 @@ timeout: sending signal INT to command 'env'
 timeout: sending signal KILL to command 'env'
 EOF
 
-env timeout --verbose --kill-after=1 --signal=INT 1 \
+env timeout --verbose --kill-after=.1 --signal=INT .1 \
     env --ignore-signal=INT \
-    seq inf > /dev/null 2>err6t
+    sleep 10 > /dev/null 2>err6t
 
 # check only the first two lines from stderr, which are printed by timeout.
 # (operating systems might add more messages, like "killed").
@@ -131,9 +125,9 @@ compare exp-err6 err6 || fail=1
 # Repeat the previous test with "--ignore-signals" and no signal names,
 # i.e., all signals.
 
-env timeout --verbose --kill-after=1 --signal=INT 1 \
+env timeout --verbose --kill-after=.1 --signal=INT .1 \
     env --ignore-signal \
-    seq inf > /dev/null 2>err7t
+    sleep 10 > /dev/null 2>err7t
 
 # check only the first two lines from stderr, which are printed by timeout.
 # (operating systems might add more messages, like "killed").
@@ -142,5 +136,4 @@ sed -n '1,2p' err7t > err7 || framework_failure_
 compare exp-err6 err7 || fail=1

-
 Exit $fail
-- 
2.9.3