Subject: bug#68708: [PATCH] env,kill: Handle unnamed signals

From: Grisha Levit <[email-masked]>
Date: Thu, 25 Jan 2024 14:52:50 -0500

> On Thu, Jan 25, 2024, 09:50 Pádraig Brady <[email-masked]> 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