GNU bug report logs - #68708
[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 68708 in the body.
You can then email your comments to 68708 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
Subject: [PATCH] env,kill: Handle unnamed signals
Date: Wed, 24 Jan 2024 15:40:31 -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

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Grisha Levit <grishalevit <at> gmail.com>, 68708 <at> debbugs.gnu.org
Subject: Re: bug#68708: [PATCH] env,kill: Handle unnamed signals
Date: Thu, 25 Jan 2024 14:50:07 +0000

On 24/01/2024 20:40, Grisha Levit wrote:
> 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
> 

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

thanks!
Pádraig

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

From: Grisha Levit <grishalevit <at> gmail.com>
To: 68708 <at> debbugs.gnu.org
Cc: P <at> draigbrady.com, Grisha Levit <grishalevit <at> gmail.com>
Subject: [PATCH] env,kill: Handle unnamed signals
Date: Thu, 25 Jan 2024 14:52:50 -0500

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

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Grisha Levit <grishalevit <at> gmail.com>, 68708 <at> debbugs.gnu.org
Subject: Re: bug#68708: [PATCH] env,kill: Handle unnamed signals
Date: Wed, 13 Mar 2024 19:22:07 +0000

[Message part 1 (text/plain, inline)]

On 25/01/2024 19:52, Grisha Levit wrote:
>> On Thu, Jan 25, 2024, 09:50 Pádraig Brady <P <at> 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.

I've made a few adjustments:

- Clarified the commit message.

- I've gone back to have kill -l produce a bare number for unnamed signals, because:
this is consistent with util-linux,
SIG%d is only parseable by coreutils (not util-linux or bash),
easier to programatically determine if a name is defined.
I've left as SIG%d for -t output as that's a coreutils specific option,
and not really programatically consumable anyway.

- I've added a validation check for `env --block=32` so that it fails
like `env --default=32` and `env --ignore=32`.
I.e. exits with EXIT_CANCELED.

- Added a NEWS entry.

I'll apply this later.

thanks!
Pádraig

[0001-env-kill-timeout-support-unnamed-signals.patch (text/x-patch, attachment)]

Previous Next

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