On 02/12/2025 16:49, Collin Funk wrote:
Pádraig Brady <[email protected]> writes:

On 02/12/2025 05:22, Collin Funk wrote:
FreeBSD added sig2str which makes tests/timeout/timeout.sh from
coreutils fail with the following:
      + compare_ exp err
      + LC_ALL=C diff -u exp err
      --- exp   2025-12-01 21:18:13.335894000 -0800
      +++ err   2025-12-01 21:18:13.790449000 -0800
      @@ -1,2 +1,2 @@
      -timeout: sending signal EXIT to command 'sleep'
      +timeout: sending signal 0 to command 'sleep'
       timeout: sending signal KILL to command 'sleep'
POSIX states [1]:
     If signum is equal to 0, the behavior is unspecified.
This seems like an uncommon use of this function, so I am tempted to
push this change to coreutils:
diff --git a/src/timeout.c b/src/timeout.c
index 7634323d4..cea997161 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -226,7 +226,9 @@ cleanup (int sig)
         if (verbose)
           {
             char signame[MAX (SIG2STR_MAX, INT_BUFSIZE_BOUND (int))];
-          if (sig2str (sig, signame) != 0)
+          if (sig == 0)
+            strcpy (signame, "EXIT");
+          else if (sig2str (sig, signame) != 0)
               snprintf (signame, sizeof signame, "%d", sig);
             error (0, 0, _("sending signal %s to command %s"),
                    signame, quote (command));
However, I figured it was best to get others opinions first. Is it
worth
replacing the system sig2str in this case?

The system sig2str is fine.

I'd probably prefer "0" rather than "EXIT" in the timeout.c code actually.
I.e. use: if (sig == 0 || sig2str (sig, signame) != 0)

EXIT comes from `trap foo 0` `trap foo EXIT` being equivalent,
so it's probably slightly better for timeout to use "0" here.

Interesting. My preference is the opposite since I would always write
'trap foo EXIT'. Tiny exception for very old shells that don't support
signal names for 'trap'.

Also my change makes it compatible with what FreeBSD < 15.0 did. Not
that I expect it to cause any breakage.

Sure, I also prefer `trap foo EXIT` in the _shell_ context.
However in the timeout(1) context, EXIT might be confusing,
as sending SIGEXIT to a process will not in fact cause it to exit.

cheers,
Padraig

Reply via email to