On 05/04/2025 09:03, Paul Eggert wrote:
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.
This is the first time coreutils is linking with libm.
So practically that does add measurable overhead:
$ time seq 10000 | xargs -n1 src/timeout-old >/dev/null
real 0m7.139s
$ time seq 10000 | xargs -n1 src/timeout >/dev/null
real 0m8.161s
So on my 2.6GHz i7-5600U we're adding 102,200 ns to the startup time,
to address theoretical single nanosecond issues.
Given the code is also more complex, I'm thinking using libm is not worth it?
BTW I'll apply the attached cleanups if we keep the libm dependency.
cheers,
Pádraig
From 4368d21fbddaf5397087210c49b398414e02c933 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 5 Apr 2025 11:27:32 +0100
Subject: [PATCH] maint: adjustments to recent timeout change
* .gitignore: Add /lib/fenv.h to ignore list.
* tests/timeout/timeout-parameters.sh: Use a sleep length of 10s
to be consistent with the pattern where we use this larger time
when it does not slow down a test, but also provides protection
against a hung test, and better avoidance of false failures due
to races on very loaded systems. Also fix the setting of FAIL.
* tests/timeout/timeout-large-parameters.sh: Remove duplicated test.
---
.gitignore | 1 +
tests/timeout/timeout-large-parameters.sh | 3 ---
tests/timeout/timeout-parameters.sh | 2 +-
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/.gitignore b/.gitignore
index f4a17ad04..4dc39ae55 100644
--- a/.gitignore
+++ b/.gitignore
@@ -67,6 +67,7 @@
/lib/errno.h
/lib/error.h
/lib/fcntl.h
+/lib/fenv.h
/lib/float.h
/lib/fnmatch.h
/lib/getopt-cdefs.h
diff --git a/tests/timeout/timeout-large-parameters.sh b/tests/timeout/timeout-large-parameters.sh
index a5395d153..5669810e8 100755
--- a/tests/timeout/timeout-large-parameters.sh
+++ b/tests/timeout/timeout-large-parameters.sh
@@ -43,7 +43,4 @@ timeout 2.34e+5d sleep 0 || fail=1
timeout $LDBL_MAX sleep 0 || fail=1
returns_ 125 timeout -- -$LDBL_MAX sleep 0 || fail=1
-# Ensure underflow times out immediately
-returns_ 124 timeout 1e-5000 sleep 10 || fail=1
-
Exit $fail
diff --git a/tests/timeout/timeout-parameters.sh b/tests/timeout/timeout-parameters.sh
index 408846262..c4b2202b4 100755
--- a/tests/timeout/timeout-parameters.sh
+++ b/tests/timeout/timeout-parameters.sh
@@ -41,7 +41,7 @@ timeout 10.34 sleep 0 || fail=1
timeout 9.999999999 sleep 0 || fail=1
# round underflow up to 1 ns
-returns_ 124 timeout 1e-10000 sleep 1 || fail
+returns_ 124 timeout 1e-10000 sleep 10 || fail=1
# invalid signal spec
returns_ 125 timeout --signal=invalid 1 sleep 0 || fail=1
--
2.48.1
