Thanks for reporting that. I merged that into some other timeout stuff I was already looking at, and pushed the following set of patches. They also should fix Bug#9076, so I'm CC'ing that and marking both bugs "done".
I have a couple of other ideas for improving "timeout" but figure they could use some review so I'll post them as separate bug reports. >From bbd4c9edfa4bd5650106261b7d9b1dd434d91581 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Fri, 15 Jul 2011 17:38:32 -0700 Subject: [PATCH 1/6] dd: port to NonStop (Bug#9076) * src/dd.c (SA_RESETHAND): Define to 0 if not defined. --- src/dd.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/dd.c b/src/dd.c index 3e75412..0824f6c 100644 --- a/src/dd.c +++ b/src/dd.c @@ -55,6 +55,11 @@ # endif #endif +/* NonStop circa 2011 lacks SA_RESETHAND; see Bug#9076. */ +#ifndef SA_RESETHAND +# define SA_RESETHAND 0 +#endif + #ifndef SIGINFO # define SIGINFO SIGUSR1 #endif -- 1.7.4.4 >From 995299ff155bef70a3b153dc8cef11ed9b1d8904 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Fri, 15 Jul 2011 17:39:28 -0700 Subject: [PATCH 2/6] ls: port to NonStop (Bug#9076) * src/ls.c (SA_RESTART): Define to 0 if not defined. --- src/ls.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/ls.c b/src/ls.c index c604e14..680a7c3 100644 --- a/src/ls.c +++ b/src/ls.c @@ -74,6 +74,14 @@ # endif #endif +/* NonStop circa 2011 lacks both SA_RESTART and siginterrupt, so don't + restart syscalls after a signal handler fires. This may cause + colors to get messed up on the screen if 'ls' is interrupted, but + that's the best we can do on such a platform. */ +#ifndef SA_RESTART +# define SA_RESTART 0 +#endif + #include "system.h" #include <fnmatch.h> -- 1.7.4.4 >From 638e670d76b3bf573f6a9930b376811b5663881a Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Fri, 15 Jul 2011 17:48:38 -0700 Subject: [PATCH 3/6] timeout: port to NonStop (Bug#9077) * src/timeout.c (SA_RESTART): Define to 0 if not defined. (main): Don't assume signal handling uses SA_RESTART. --- src/timeout.c | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/timeout.c b/src/timeout.c index ef660a7..895d720 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -64,6 +64,11 @@ # include <sys/resource.h> #endif +/* NonStop circa 2011 lacks both SA_RESTART and siginterrupt. */ +#ifndef SA_RESTART +# define SA_RESTART 0 +#endif + #define PROGRAM_NAME "timeout" #define AUTHORS proper_name_utf8 ("Padraig Brady", "P\303\241draig Brady") @@ -256,7 +261,8 @@ install_signal_handlers (int sigterm) struct sigaction sa; sigemptyset (&sa.sa_mask); /* Allow concurrent calls to handler */ sa.sa_handler = cleanup; - sa.sa_flags = SA_RESTART; /* restart syscalls (like wait() below) */ + sa.sa_flags = SA_RESTART; /* Restart syscalls if possible, as that's + more likely to work cleanly. */ sigaction (SIGALRM, &sa, NULL); /* our timeout. */ sigaction (SIGINT, &sa, NULL); /* Ctrl-C at terminal for example. */ @@ -354,18 +360,15 @@ main (int argc, char **argv) } else { + pid_t wait_result; int status; alarm (timeout); - /* We're just waiting for a single process here, so wait() suffices. - Note the signal() calls above on GNU/Linux and BSD at least, - essentially call the lower level sigaction() with the SA_RESTART flag - set, which ensures the following wait call will only return if the - child exits, not on this process receiving a signal. Also we're not - passing WUNTRACED | WCONTINUED to a waitpid() call and so will not get - indication that the child has stopped or continued. */ - if (wait (&status) == -1) + while ((wait_result = wait (&status)) < 0 && errno == EINTR) + continue; + + if (wait_result < 0) { /* shouldn't happen. */ error (0, errno, _("error waiting for command")); -- 1.7.4.4 >From 1f97794a3acfad6d169c60adec5a7ce916baa8f9 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Sat, 16 Jul 2011 05:57:19 -0700 Subject: [PATCH 4/6] * src/timeout.c (main): Use waitpid, not wait (Bug#9098). Reported by Andreas Schwab. * src/timeout.c (SA_RESTART): Define to 0 if not defined. --- src/timeout.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/timeout.c b/src/timeout.c index 895d720..2d6dad8 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -365,7 +365,8 @@ main (int argc, char **argv) alarm (timeout); - while ((wait_result = wait (&status)) < 0 && errno == EINTR) + while ((wait_result = waitpid (monitored_pid, &status, 0)) < 0 + && errno == EINTR) continue; if (wait_result < 0) -- 1.7.4.4 >From 22e9697c795148548410673260595745d4e8d764 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Sat, 16 Jul 2011 06:03:47 -0700 Subject: [PATCH 5/6] Fix capiTalization in comments. --- src/timeout.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/timeout.c b/src/timeout.c index 2d6dad8..33bb8e4 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -35,7 +35,7 @@ This can be seen with `timeout 10 dd&` for example. However if one brings this group to the foreground with the `fg` command before the timer expires, the command will remain - in the sTop state as the shell doesn't send a SIGCONT + in the stop state as the shell doesn't send a SIGCONT because the timeout process (group leader) is already running. To get the command running again one can Ctrl-Z, and do fg again. Note one can Ctrl-C the whole job when in this state. @@ -333,9 +333,9 @@ main (int argc, char **argv) /* Setup handlers before fork() so that we handle any signals caused by child, without races. */ install_signal_handlers (term_signal); - signal (SIGTTIN, SIG_IGN); /* don't sTop if background child needs tty. */ - signal (SIGTTOU, SIG_IGN); /* don't sTop if background child needs tty. */ - signal (SIGCHLD, SIG_DFL); /* Don't inherit CHLD handling from parent. */ + signal (SIGTTIN, SIG_IGN); /* Don't stop if background child needs tty. */ + signal (SIGTTOU, SIG_IGN); /* Don't stop if background child needs tty. */ + signal (SIGCHLD, SIG_DFL); /* Don't inherit CHLD handling from parent. */ monitored_pid = fork (); if (monitored_pid == -1) -- 1.7.4.4 >From f9bcef4765dc7c3ecb8d7c58ae410087195e6fb1 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Sat, 16 Jul 2011 12:07:46 -0700 Subject: [PATCH 6/6] timeout: treat seconds counts like 'sleep' does Treat fractions as a request to round up to the next representable value, and treat out-of-range values as maximal ones. This is consistent with how "sleep" works. And this way, "timeout 999999999999999999d FOO" and "timeout 4.5 foo" are more likely to do what the user wants. * src/timeout.c: Include c-strtod.h and xstrtod.h, not xstrtol.h. (apply_time_suffix): Change it to the way sleep.c's time_suffix does things. Maybe this function (identical in both programs, other than its name) should be moved to a library? (parse_duration): Return a maximal value on overflow. Return unsigned int, not unsigned long. Allow fractions, which round up to the next integer value. * tests/misc/timeout-parameters: Adjust tests to match new behavior. Add a very large number. --- src/timeout.c | 60 ++++++++++++++++++++++++----------------- tests/misc/timeout-parameters | 16 +++++++++- 2 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/timeout.c b/src/timeout.c index 33bb8e4..ccb4f85 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -52,7 +52,8 @@ #include <sys/wait.h> #include "system.h" -#include "xstrtol.h" +#include "c-strtod.h" +#include "xstrtod.h" #include "sig2str.h" #include "operand2sig.h" #include "error.h" @@ -196,54 +197,51 @@ use the KILL (9) signal, since this signal cannot be caught.\n"), stdout); exit (status); } -/* Given a long integer value *X, and a suffix character, SUFFIX_CHAR, +/* Given a floating point value *X, and a suffix character, SUFFIX_CHAR, scale *X by the multiplier implied by SUFFIX_CHAR. SUFFIX_CHAR may be the NUL byte or `s' to denote seconds, `m' for minutes, `h' for hours, or `d' for days. If SUFFIX_CHAR is invalid, don't modify *X - and return false. If *X would overflow an integer, don't modify *X - and return false. Otherwise return true. */ + and return false. Otherwise return true. */ static bool -apply_time_suffix (unsigned long *x, char suffix_char) +apply_time_suffix (double *x, char suffix_char) { - unsigned int multiplier = 1; + int multiplier; switch (suffix_char) { case 0: case 's': - return true; - case 'd': - multiplier *= 24; - case 'h': - multiplier *= 60; + multiplier = 1; + break; case 'm': - if (multiplier > UINT_MAX / 60) /* 16 bit overflow */ - return false; - multiplier *= 60; + multiplier = 60; + break; + case 'h': + multiplier = 60 * 60; + break; + case 'd': + multiplier = 60 * 60 * 24; break; default: return false; } - if (*x > UINT_MAX / multiplier) - return false; - *x *= multiplier; return true; } -static unsigned long +static unsigned int parse_duration (const char* str) { - unsigned long duration; - char *ep; + double duration; + const char *ep; - if (xstrtoul (str, &ep, 10, &duration, NULL) - /* Invalid interval. Note 0 disables timeout */ - || (duration > UINT_MAX) - /* Extra chars after the number and an optional s,m,h,d char. */ + if (!xstrtod (str, &ep, &duration, c_strtod) + /* Nonnegative interval. */ + || ! (0 <= duration) + /* No extra chars after the number and an optional s,m,h,d char. */ || (*ep && *(ep + 1)) /* Check any suffix char and update timeout based on the suffix. */ || !apply_time_suffix (&duration, *ep)) @@ -252,7 +250,19 @@ parse_duration (const char* str) usage (EXIT_CANCELED); } - return duration; + /* Return the requested duration, rounded up to the next representable value. + Treat out-of-range values as if they were maximal, + as that's more useful in practice than reporting an error. + + FIXME: Use dtotimespec + setitimer if setitimer is available, + as that has higher resolution. */ + if (UINT_MAX <= duration) + return UINT_MAX; + else + { + unsigned int duration_floor = duration; + return duration_floor + (duration_floor < duration); + } } static void diff --git a/tests/misc/timeout-parameters b/tests/misc/timeout-parameters index 6e9152b..56804a8 100755 --- a/tests/misc/timeout-parameters +++ b/tests/misc/timeout-parameters @@ -37,11 +37,23 @@ test $? = 125 || fail=1 # timeout overflow timeout $UINT_OFLOW sleep 0 -test $? = 125 || fail=1 +test $? = 0 || fail=1 # timeout overflow timeout $(expr $UINT_MAX / 86400 + 1)d sleep 0 -test $? = 125 || fail=1 +test $? = 0 || fail=1 + +# timeout overflow +timeout 999999999999999999999999999999999999999999999999999999999999d sleep 0 +test $? = 0 || fail=1 + +# floating point notation +timeout 2.34 sleep 0 +test $? = 0 || fail=1 + +# floating point notation +timeout 2.34e+5d sleep 0 +test $? = 0 || fail=1 # invalid signal spec timeout --signal=invalid 1 sleep 0 -- 1.7.4.4
