> On Thu, Jan 25, 2024, 09:50 Pádraig Brady <p...@draigbrady.com> wrote:
> This mostly looks good, except:
>
> - No need to clear the errno before kill(3).
> - Better to use SIG%d rather than the bare %d for signal _names_, as we
> already parse this format
Makes sense, done below.
* src/operand2sig.c (operand2sig): Drop signame argument, accept all
signal numbers <= SIGNUM_BOUND. All callers updated.
* src/env.c (parse_signal_action_params, reset_signal_handlers)
(parse_block_signal_params, set_signal_proc_mask)
(list_signal_handling): Accept all signal numbers <= SIGNUM_BOUND,
use SIG%d for printing if necessary.
* src/kill.c (list_signals, main): Likewise.
(send_signals): Check errno from kill(3) for bad signo.
* src/timeout.c (main): Update operand2sig call.
* tests/misc/kill.sh: Test listing all signal numbers.
---
src/env.c | 18 +++++++++---------
src/kill.c | 26 +++++++++++++++++---------
src/operand2sig.c | 8 ++++----
src/operand2sig.h | 2 +-
src/timeout.c | 3 +--
tests/misc/kill.sh | 8 ++++++++
6 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/src/env.c b/src/env.c
index ffe896039..c73a4f70a 100644
--- a/src/env.c
+++ b/src/env.c
@@ -538,7 +538,6 @@ parse_split_string (char const *str, int *orig_optind,
static void
parse_signal_action_params (char const *optarg, bool set_default)
{
- char signame[SIG2STR_MAX];
char *opt_sig;
char *optarg_writable;
@@ -548,8 +547,7 @@ parse_signal_action_params (char const *optarg, bool
set_default)
Some signals cannot be set to ignore or default (e.g., SIGKILL,
SIGSTOP on most OSes, and SIGCONT on AIX.) - so ignore errors. */
for (int i = 1 ; i <= SIGNUM_BOUND; i++)
- if (sig2str (i, signame) == 0)
- signals[i] = set_default ? DEFAULT_NOERR : IGNORE_NOERR;
+ signals[i] = set_default ? DEFAULT_NOERR : IGNORE_NOERR;
return;
}
@@ -558,7 +556,7 @@ parse_signal_action_params (char const *optarg, bool
set_default)
opt_sig = strtok (optarg_writable, ",");
while (opt_sig)
{
- int signum = operand2sig (opt_sig, signame);
+ int signum = operand2sig (opt_sig);
/* operand2sig accepts signal 0 (EXIT) - but we reject it. */
if (signum == 0)
error (0, 0, _("%s: invalid signal"), quote (opt_sig));
@@ -607,7 +605,8 @@ reset_signal_handlers (void)
if (dev_debug)
{
char signame[SIG2STR_MAX];
- sig2str (i, signame);
+ if (sig2str (i, signame) != 0)
+ snprintf (signame, sizeof signame, "SIG%d", i);
devmsg ("Reset signal %s (%d) to %s%s\n",
signame, i,
set_to_default ? "DEFAULT" : "IGNORE",
@@ -620,7 +619,6 @@ reset_signal_handlers (void)
static void
parse_block_signal_params (char const *optarg, bool block)
{
- char signame[SIG2STR_MAX];
char *opt_sig;
char *optarg_writable;
@@ -647,7 +645,7 @@ parse_block_signal_params (char const *optarg, bool block)
opt_sig = strtok (optarg_writable, ",");
while (opt_sig)
{
- int signum = operand2sig (opt_sig, signame);
+ int signum = operand2sig (opt_sig);
/* operand2sig accepts signal 0 (EXIT) - but we reject it. */
if (signum == 0)
error (0, 0, _("%s: invalid signal"), quote (opt_sig));
@@ -695,7 +693,8 @@ set_signal_proc_mask (void)
if (dev_debug && debug_act)
{
char signame[SIG2STR_MAX];
- sig2str (i, signame);
+ if (sig2str (i, signame) != 0)
+ snprintf (signame, sizeof signame, "SIG%d", i);
devmsg ("signal %s (%d) mask set to %s\n",
signame, i, debug_act);
}
@@ -728,7 +727,8 @@ list_signal_handling (void)
if (! *ignored && ! *blocked)
continue;
- sig2str (i, signame);
+ if (sig2str (i, signame) != 0)
+ snprintf (signame, sizeof signame, "SIG%d", i);
fprintf (stderr, "%-10s (%2d): %s%s%s\n", signame, i,
blocked, connect, ignored);
}
diff --git a/src/kill.c b/src/kill.c
index 9c8b6c191..d6aeae0b9 100644
--- a/src/kill.c
+++ b/src/kill.c
@@ -131,11 +131,15 @@ list_signals (bool table, char *const *argv)
if (argv)
for (; *argv; argv++)
{
- signum = operand2sig (*argv, signame);
+ signum = operand2sig (*argv);
if (signum < 0)
status = EXIT_FAILURE;
else
- print_table_row (num_width, signum, name_width, signame);
+ {
+ if (sig2str (signum, signame) != 0)
+ snprintf (signame, sizeof signame, "SIG%d", signum);
+ print_table_row (num_width, signum, name_width, signame);
+ }
}
else
for (signum = 1; signum <= SIGNUM_BOUND; signum++)
@@ -147,16 +151,18 @@ list_signals (bool table, char *const *argv)
if (argv)
for (; *argv; argv++)
{
- signum = operand2sig (*argv, signame);
+ signum = operand2sig (*argv);
if (signum < 0)
status = EXIT_FAILURE;
- else
+ else if (ISDIGIT (**argv))
{
- if (ISDIGIT (**argv))
+ if (sig2str (signum, signame) == 0)
puts (signame);
else
- printf ("%d\n", signum);
+ printf ("SIG%d\n", signum);
}
+ else
+ printf ("%d\n", signum);
}
else
for (signum = 1; signum <= SIGNUM_BOUND; signum++)
@@ -190,7 +196,10 @@ send_signals (int signum, char *const *argv)
}
else if (kill (pid, signum) != 0)
{
- error (0, errno, "%s", quote (arg));
+ if (errno == EINVAL)
+ error (0, errno, "%d", signum);
+ else
+ error (0, errno, "%s", quote (arg));
status = EXIT_FAILURE;
}
}
@@ -206,7 +215,6 @@ main (int argc, char **argv)
bool list = false;
bool table = false;
int signum = -1;
- char signame[SIG2STR_MAX];
initialize_main (&argc, &argv);
set_program_name (argv[0]);
@@ -251,7 +259,7 @@ main (int argc, char **argv)
error (0, 0, _("%s: multiple signals specified"), quote (optarg));
usage (EXIT_FAILURE);
}
- signum = operand2sig (optarg, signame);
+ signum = operand2sig (optarg);
if (signum < 0)
usage (EXIT_FAILURE);
break;
diff --git a/src/operand2sig.c b/src/operand2sig.c
index 2a2563c62..b46cb1bed 100644
--- a/src/operand2sig.c
+++ b/src/operand2sig.c
@@ -18,8 +18,8 @@
FIXME: Move this to gnulib/str2sig.c */
-/* Convert OPERAND to a signal number with printable representation SIGNAME.
- Return the signal number, or -1 if unsuccessful. */
+/* Convert OPERAND to a signal number. Return the signal number, or -1 if
+ unsuccessful. */
#include <config.h>
#include <stdio.h>
@@ -32,7 +32,7 @@
#include "operand2sig.h"
extern int
-operand2sig (char const *operand, char *signame)
+operand2sig (char const *operand)
{
int signum;
@@ -82,7 +82,7 @@ operand2sig (char const *operand, char *signame)
free (upcased);
}
- if (signum < 0 || sig2str (signum, signame) != 0)
+ if (0 > signum || signum > SIGNUM_BOUND)
{
error (0, 0, _("%s: invalid signal"), quote (operand));
return -1;
diff --git a/src/operand2sig.h b/src/operand2sig.h
index e46689e7b..3bc551051 100644
--- a/src/operand2sig.h
+++ b/src/operand2sig.h
@@ -15,5 +15,5 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <https://www.gnu.org/licenses/>. */
-extern int operand2sig (char const *operand, char *signame)
+extern int operand2sig (char const *operand)
_GL_ATTRIBUTE_NONNULL ();
diff --git a/src/timeout.c b/src/timeout.c
index 85d97c0b5..7d1ea7da6 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -462,7 +462,6 @@ int
main (int argc, char **argv)
{
double timeout;
- char signame[SIG2STR_MAX];
int c;
initialize_main (&argc, &argv);
@@ -483,7 +482,7 @@ main (int argc, char **argv)
break;
case 's':
- term_signal = operand2sig (optarg, signame);
+ term_signal = operand2sig (optarg);
if (term_signal == -1)
usage (EXIT_CANCELED);
break;
diff --git a/tests/misc/kill.sh b/tests/misc/kill.sh
index 69679e5a6..79a93de5f 100755
--- a/tests/misc/kill.sh
+++ b/tests/misc/kill.sh
@@ -60,4 +60,12 @@ returns_ 1 env kill -l -1 || fail=1
returns_ 1 env kill -l -1 0 || fail=1
returns_ 1 env kill -l INVALID TERM || fail=1
+# Verify all signal numbers can be listed
+SIG_LAST_STR=$(env kill -l | tail -n1) || framework_failure_
+SIG_LAST_NUM=$(env kill -l -- "$SIG_LAST_STR") || framework_failure_
+SIG_SEQ=$(env seq -- 0 "$SIG_LAST_NUM") || framework_failure_
+test -n "$SIG_SEQ" || framework_failure_
+env kill -l -- $SIG_SEQ || fail=1
+env kill -t -- $SIG_SEQ || fail=1
+
Exit $fail
--
2.43.0
