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?= <p...@draigbrady.com>
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
