On 2025-04-04 12:27, Pádraig Brady wrote:
+ /* Clamp underflow to 1ns, as 0 disables the timeout. */
+ if (duration == 0 && errno == ERANGE)
+ duration = 1e-9;
That isn't exactly right as the 1e-9 double-rounds to 2e-9 when we
compute the struct timespec. Also, even with the patch the code
mishandles 16777216.000000001 (2**24 + 10**-9) by treating it as if it
were just 16777216. These are tiny bugs, I know, but I took up the
challenge of doing this things more correctly by installing the attached.From a3b862ece2aa04d5c0b5e93fec8e66e45b9b3560 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 5 Apr 2025 00:58:22 -0700
Subject: [PATCH] timeout: round timeouts up
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This handles timeouts like 16777216.000000001 correctly;
formerly the subsecond part of that timeout was ignored.
* bootstrap.conf (gnulib_modules): Add fenv-rounding, signbit.
* src/local.mk (src_timeout_LDADD): Append $(FENV_ROUNDING_LIBM).
* src/timeout.c: Include fenv.h, math.h.
Don’t include xstrtod.h, as xstrtod’s checking now gets in the way.
(parse_duration): Round up when calling cl_strtod.
Check for -1e-1000. Don’t double-round 1e-9.
* tests/timeout/timeout-parameters.sh: Test for -0.1,
-1e-1000, 1e-1000.
---
bootstrap.conf | 2 ++
src/local.mk | 3 +++
src/timeout.c | 39 ++++++++++++++++++++++++-----
tests/timeout/timeout-parameters.sh | 7 +++++-
4 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index c99838e95..3c133ef71 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -99,6 +99,7 @@ gnulib_modules="
fdatasync
fdopen
fdutimensat
+ fenv-rounding
file-has-acl
file-type
fileblocks
@@ -242,6 +243,7 @@ gnulib_modules="
settime
sig2str
sigaction
+ signbit
skipchars
smack
ssize_t
diff --git a/src/local.mk b/src/local.mk
index fd9dc81c2..0a4e3af43 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -257,6 +257,9 @@ src_stat_LDADD += $(LIB_SELINUX)
# for nvlist_lookup_uint64_array
src_stat_LDADD += $(LIB_NVPAIR)
+# for fegetround, fesetround
+src_timeout_LDADD += $(FENV_ROUNDING_LIBM)
+
# for gettime, settime, tempname, utimecmp, utimens
copy_ldadd += $(CLOCK_TIME_LIB)
src_date_LDADD += $(CLOCK_TIME_LIB)
diff --git a/src/timeout.c b/src/timeout.c
index 6756cd888..5947c1942 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -45,7 +45,9 @@
Written by Pádraig Brady. */
#include <config.h>
+#include <fenv.h>
#include <getopt.h>
+#include <math.h>
#include <stdio.h>
#include <sys/types.h>
#include <signal.h>
@@ -56,7 +58,6 @@
#include "system.h"
#include "cl-strtod.h"
-#include "xstrtod.h"
#include "sig2str.h"
#include "operand2sig.h"
#include "quote.h"
@@ -356,12 +357,29 @@ apply_time_suffix (double *x, char suffix_char)
static double
parse_duration (char const *str)
{
- double duration;
- char const *ep;
+ /* If possible tell strtod to round up, so that we always wait at
+ least as long as STR specifies. Although Standard C requires
+ FENV_ACCESS ON, don't bother if using GCC as it warns. */
+#ifdef FE_UPWARD
+# if !defined __GNUC__ && 199901 <= __STDC_VERSION__
+# pragma STDC FENV_ACCESS ON
+# endif
+ int round = fegetround ();
+ fesetround (FE_UPWARD);
+#endif
+
+ char *ep;
+ double duration = cl_strtod (str, &ep);
+
+#ifdef FE_UPWARD
+ fesetround (round);
+#endif
- if (! (xstrtod (str, &ep, &duration, cl_strtod) || errno == ERANGE)
+ if (ep == str
/* Nonnegative interval. */
|| ! (0 <= duration)
+ /* The interval did not underflow to -0. */
+ || (signbit (duration) && errno == ERANGE)
/* 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. */
@@ -371,9 +389,18 @@ parse_duration (char const *str)
usage (EXIT_CANCELED);
}
- /* Clamp underflow to 1ns, as 0 disables the timeout. */
+ /* Do not let the duration underflow to 0, as 0 disables the timeout.
+ Use 2**-30 instead of 0; settimeout will round it up to 1 ns,
+ whereas 1e-9 might double-round to 2 ns.
+
+ If FE_UPWARD is defined, this code is not needed on glibc as its
+ strtod rounds upward correctly. Although this code might not be
+ needed on non-glibc platforms too, it's too much trouble to
+ worry about that. */
+#if !defined FE_UPWARD || !defined __GLIBC__
if (duration == 0 && errno == ERANGE)
- duration = 1e-9;
+ duration = 9.313225746154785e-10;
+#endif
return duration;
}
diff --git a/tests/timeout/timeout-parameters.sh b/tests/timeout/timeout-parameters.sh
index 84b450e90..408846262 100755
--- a/tests/timeout/timeout-parameters.sh
+++ b/tests/timeout/timeout-parameters.sh
@@ -23,8 +23,10 @@ getlimits_
# internal errors are 125, distinct from execution failure
-# invalid timeout
+# invalid timeouts
returns_ 125 timeout invalid sleep 0 || fail=1
+returns_ 125 timeout ' -0.1' sleep 0 || fail=1
+returns_ 125 timeout ' -1e-10000' sleep 0 || fail=1
# invalid kill delay
returns_ 125 timeout --kill-after=invalid 1 sleep 0 || fail=1
@@ -38,6 +40,9 @@ timeout 10.34 sleep 0 || fail=1
# nanoseconds potentially supported
timeout 9.999999999 sleep 0 || fail=1
+# round underflow up to 1 ns
+returns_ 124 timeout 1e-10000 sleep 1 || fail
+
# invalid signal spec
returns_ 125 timeout --signal=invalid 1 sleep 0 || fail=1
--
2.45.2
