GNU bug report logs - #68707
[PATCH] env,kill: Handle unnamed signals

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 68707 in the body.
You can then email your comments to 68707 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Grisha Levit <grishalevit <at> gmail.com>
To: bug-coreutils <at> gnu.org
Cc: Grisha Levit <grishalevit <at> gmail.com>
Subject: [PATCH] env,kill: Handle unnamed signals
Date: Wed, 24 Jan 2024 14:32:48 -0500

Android reserves [1] some realtime signals and redefines [2] SIGRTMIN,
 leaving a gap between the signals that have SIG* constants defined in
 signal.h and SIGRTMIN.
 
 When passed such a signal number, gnulib sig2str returns -1 and leaves
 its signame argument unchanged.
 
 The signal listing in env ends up reusing the name of the last printed
 valid signal:
 
     $ env --list-signal-handling true
     HUP        ( 1): IGNORE
     HUP        (32): BLOCK
     HUP        (38): IGNORE
 
 ..and the corresponding signal numbers are rejected as operands for the
 env, kill, and timeout commands.
 
 This patch removes the requirement that sig2str returns 0 for a signal
 number associated with an operand, and allows unnamed signals to be in
 the sets `env' attempts to manipulate when a --*-signal option is used
 with no argument.
 
 This does leave the possibility of numbers lower than SIGNUM_BOUND that
 are not valid signals, but I'm not sure that's really a problem for the
 existing code paths (if it is, adding checks for sigset_t manipulations
 would probably be enough).
 
 To be on the safe side, added a check to report kill(3) EINVAL as a bad
 signo (rather than always blaming the pid).
 
 This does not change the default list printed with `kill -l'.  When a
 name is to be printed, the signal number associated with an unnamed
 signal is used.
 
 [1]: https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/platform/bionic/reserved_signals.h
 [2]: https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/include/signal.h;l=51


* 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 signal number 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         | 29 ++++++++++++++++-------------
 src/operand2sig.c  |  8 ++++----
 src/operand2sig.h  |  2 +-
 src/timeout.c      |  3 +--
 tests/misc/kill.sh |  8 ++++++++
 6 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/src/env.c b/src/env.c
index ffe896039..e40767610 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, "%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, "%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, "%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..f386c9cbc 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, "%d", signum);
+                print_table_row (num_width, signum, name_width, signame);
+              }
           }
       else
         for (signum = 1; signum <= SIGNUM_BOUND; signum++)
@@ -147,16 +151,13 @@ 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 if (ISDIGIT (**argv) && sig2str (signum, signame) == 0)
+              puts (signame);
             else
-              {
-                if (ISDIGIT (**argv))
-                  puts (signame);
-                else
-                  printf ("%d\n", signum);
-              }
+              printf ("%d\n", signum);
           }
       else
         for (signum = 1; signum <= SIGNUM_BOUND; signum++)
@@ -188,9 +189,12 @@ send_signals (int signum, char *const *argv)
           error (0, 0, _("%s: invalid process id"), quote (arg));
           status = EXIT_FAILURE;
         }
-      else if (kill (pid, signum) != 0)
+      else if (errno = 0, 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 +210,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 +254,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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.