On 14/01/16 03:37, Pádraig Brady wrote: >> Does the test pass when changing s/1000/2000/ on the ulimit line? >> > >>> >> FAIL: tests/misc/kill >> > >>> >> + SIGINVAL=32 >>> >> + returns_ 1 env kill -l 32 0 >>> >> LWP >>> >> EXIT >>> >> + fail=1 >> > >> > This is a new test, and the scheme used to pick an >> > invalid signal number is not general. >> > I'll see if I can get sig2str() to return -1 generally. > Looking further it seems that NSIG is only for older signals on FreeBSD > and there is a newer _SIG_MAXSIG that we can add to gnulib like: > > diff --git a/lib/sig2str.h b/lib/sig2str.h > index f347170..2730774 100644 > --- a/lib/sig2str.h > +++ b/lib/sig2str.h > @@ -44,6 +44,8 @@ int str2sig (char const *, int *); > > #if defined _sys_nsig > # define SIGNUM_BOUND (_sys_nsig - 1) > +#elif defined _SIG_MAXSIG > +# define SIGNUM_BOUND (_SIG_MAXSIG - 2) /* FreeBSD >= 7. */ > #elif defined NSIG > # define SIGNUM_BOUND (NSIG - 1) > #else
Now in gnulib. Will sync with this soon: http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=dce117ac > As for the test to get an invalid signal number, this would be more general: > > env kill -t | sort -k 1b,1 > sig.list > seq 256 | sort -k 1b,1 > all.list > invalid=$(join -v2 sig.list all.list | head -n1) > if test "$invalid"; then > ... > fi Though a bit of overkill really. > Related to this I also notice that we wrap signal specs > which doesn't seem appropriate here: > > $ bash-kill -l 257 > bash: kill: 257: invalid signal specification > $ util-linux-kill -l 257 > kill: unknown signal: 257 > $ coreutils-kill -l 257 > HUP > $ coreutils-kill -l 1 > HUP A patch for both issues above attached. thanks, Pádraig
>From cb153026f6e0870791edac8f75837f53a63153db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Thu, 14 Jan 2016 14:21:53 +0000 Subject: [PATCH] kill: restrict signal matching to valid exit status values * src/operand2sig.c (operand2sig): Limit valid numbers to possible exit status values rather than arbitrary wait(2) status values and beyond. * tests/misc/kill.sh: Add a test case while also simplifying the INVALID signal number determination which also avoids a false failure on systems like FreeBSD 10 with incomplete signal list (caused by inaccurate NSIG). * NEWS: Mention the change in behavior. --- NEWS | 4 ++++ src/operand2sig.c | 3 ++- tests/misc/kill.sh | 10 ++++++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index b42da0a..6e1c45c 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,10 @@ GNU coreutils NEWS -*- outline -*- ls no longer prematurely wraps lines when printing short file names. [bug introduced in coreutils-5.1.0] + kill and timeout now only convert exit status values that could be used + to represent a signal. For example `kill -l $((2**30+1))` now reports + an invalid signal representation, when previously it returned "HUP". + mv no longer causes data loss due to removing a source directory specified multiple times, when that directory is also specified as the destination. [bug introduced in coreutils-8.24] diff --git a/src/operand2sig.c b/src/operand2sig.c index a9fede5..15dbec7 100644 --- a/src/operand2sig.c +++ b/src/operand2sig.c @@ -40,9 +40,10 @@ operand2sig (char const *operand, char *signame) if (ISDIGIT (*operand)) { char *endp; + const int exitmax = WEXITSTATUS (~0); long int l = (errno = 0, strtol (operand, &endp, 10)); int i = l; - signum = (operand == endp || *endp || errno || i != l ? -1 + signum = (operand == endp || *endp || errno || i != l || exitmax < i ? -1 : WIFSIGNALED (i) ? WTERMSIG (i) : i); } else diff --git a/tests/misc/kill.sh b/tests/misc/kill.sh index 27d550e..4095c6e 100755 --- a/tests/misc/kill.sh +++ b/tests/misc/kill.sh @@ -19,6 +19,8 @@ . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src print_ver_ kill +getlimits_ + # params required returns_ 1 env kill || fail=1 returns_ 1 env kill -TERM || fail=1 @@ -50,8 +52,12 @@ SIGTERM=$(env kill -l HUP TERM | tail -n1) || fail=1 test $(env kill -l "$SIGTERM") = TERM || fail=1 # Verify invalid signal spec is diagnosed -SIGINVAL=$(env kill -t | tail -n1 | cut -f1 -d' ') -SIGINVAL=$(expr "$SIGINVAL" + 1) +# Pick an invalid value that is practically too wide +# given that wait(2) etc. pass everything through an int. +# Also ensure the bottom bit is set so we validate +# we don't match SIGHUP as done in coreutils <= 8.24. +SIGINVAL=$(expr $INT_MAX / 2 + 2) +returns_ 1 env kill -l "$SIGINVAL" || fail=1 returns_ 1 env kill -l "$SIGINVAL" 0 || fail=1 returns_ 1 env kill -l INVALID TERM || fail=1 -- 2.5.0
