Subject: Re: bug#77535: timeout treats very short durations as `0`

From: Paul Eggert <[email-masked]>
Date: Mon, 7 Apr 2025 01:08:43 -0700

On 2025-04-05 13:56, Pádraig Brady wrote:
> I'm going to apply the attached later to remove the libm dependency.
> It's just too much of a dependency for minimal functionality.

Thanks, agreed about the dependency. Still, 'timeout' (and, now that I 
think about it, 'sleep' and 'tail') should always wait for at least the 
time requested.

After thinking about it I installed the attached further patch, which 
addresses the problem in a different way without needing libm on 
GNU/Linux. Although we could tighten the bounds further in typical cases 
(by using the algorithm 'date' uses to parse seconds, if the string is 
of that form) I tried to keep it simple, as the main point is to not 
sleep for less than the time requested.

Floating point can be such a pain sometimes, even for simple things. 
(Read the cited paper and see....)
From cb7c210d30c5a6a4a879bda96d61926d77f16d96 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 7 Apr 2025 00:30:14 -0700
Subject: [PATCH] =?UTF-8?q?timeout:=20don=E2=80=99t=20sleep=20less=20than?=
 =?UTF-8?q?=20requested?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Also, change sleep and tail to not sleep less than requested.
* bootstrap.conf (gnulib_modules): Add dtimespec-bound.
* gl/lib/dtimespec-bound.c, gl/lib/dtimespec-bound.h:
* gl/modules/dtimespec-bound: New files.
* src/sleep.c, src/tail.c, src/timeout.c: Include dtimespec-bound.h.
* src/sleep.c, src/tail.c: Don’t include xstrtod.h.
* src/sleep.c (apply_suffix, main):
* src/tail.c (parse_options):
* src/timeout.c (apply_time_suffix):
Don’t sleep less than the true number of seconds.
* src/timeout.c: Don’t include ctype.h.
(is_negative): Remove; no longer needed.
(parse_duration): Use a slightly looser bound on the timeout, one
that doesn’t need -lm on GNU/Linux.  Clear errno before calling
cl_strtod.
---
 NEWS                       |  4 ++
 bootstrap.conf             |  1 +
 gl/lib/dtimespec-bound.c   |  3 ++
 gl/lib/dtimespec-bound.h   | 76 ++++++++++++++++++++++++++++++++++++++
 gl/modules/dtimespec-bound | 24 ++++++++++++
 src/sleep.c                | 14 ++++---
 src/tail.c                 | 10 +++--
 src/timeout.c              | 29 ++++-----------
 8 files changed, 129 insertions(+), 32 deletions(-)
 create mode 100644 gl/lib/dtimespec-bound.c
 create mode 100644 gl/lib/dtimespec-bound.h
 create mode 100644 gl/modules/dtimespec-bound

diff --git a/NEWS b/NEWS
index b4559cefc..d436f9acf 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   For example `timeout 1e-5000 sleep inf` would never timeout.
   [bug introduced with timeout in coreutils-7.0]
 
+  sleep, tail, and timeout would sometimes sleep for slightly less
+  time than requested.
+  [bug introduced in coreutils-5.0]
+
   'who -m' now outputs entries for remote logins.  Previously login
   entries prefixed with the service (like "sshd") were not matched.
   [bug introduced in coreutils-9.4]
diff --git a/bootstrap.conf b/bootstrap.conf
index c99838e95..94c164e10 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -78,6 +78,7 @@ gnulib_modules="
   dirfd
   dirname
   do-release-commit-and-tag
+  dtimespec-bound
   dtoastr
   dup2
   endian
diff --git a/gl/lib/dtimespec-bound.c b/gl/lib/dtimespec-bound.c
new file mode 100644
index 000000000..8cb7608fe
--- /dev/null
+++ b/gl/lib/dtimespec-bound.c
@@ -0,0 +1,3 @@
+#include <config.h>
+#define DTIMESPEC_BOUND_INLINE _GL_EXTERN_INLINE
+#include "dtimespec-bound.h"
diff --git a/gl/lib/dtimespec-bound.h b/gl/lib/dtimespec-bound.h
new file mode 100644
index 000000000..494714574
--- /dev/null
+++ b/gl/lib/dtimespec-bound.h
@@ -0,0 +1,76 @@
+/* Compute a timespec-related bound for floating point.
+
+   Copyright 2025 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
+
+/* written by Paul Eggert */
+
+#ifndef DTIMESPEC_BOUND_H
+#define DTIMESPEC_BOUND_H 1
+
+/* This file uses _GL_INLINE_HEADER_BEGIN, _GL_INLINE.  */
+#if !_GL_CONFIG_H_INCLUDED
+ #error "Please include config.h first."
+#endif
+
+#include <errno.h>
+#include <float.h>
+#include <math.h>
+
+_GL_INLINE_HEADER_BEGIN
+#ifndef DTIMESPEC_BOUND_INLINE
+# define DTIMESPEC_BOUND_INLINE _GL_INLINE
+#endif
+
+/* If C is positive and finite, return the least floating point value
+   greater than C.  However, if 0 < C < (2 * DBL_TRUE_MIN) / (DBL_EPSILON**2),
+   return a positive value less than 1e-9.
+
+   If C is +0.0, return a positive value < 1e-9 if ERR == ERANGE, C otherwise.
+   If C is +Inf, return C.
+   If C is negative, return -timespec_roundup(-C).
+   If C is a NaN, return a NaN.
+
+   Assume round-to-even.
+
+   This function can be useful if some floating point operation
+   rounded to C but we want a nearby bound on the true value, where
+   the bound can be converted to struct timespec.  If the operation
+   underflowed to zero, ERR should be ERANGE a la strtod.  */
+
+DTIMESPEC_BOUND_INLINE double
+dtimespec_bound (double c, int err)
+{
+  /* Do not use copysign or nextafter, as they link to -lm in GNU/Linux.  */
+
+  /* Use DBL_TRUE_MIN for the special case of underflowing to zero;
+     any positive value less than 1e-9 will do.  */
+  if (err == ERANGE && c == 0)
+    return signbit (c) ? -DBL_TRUE_MIN : DBL_TRUE_MIN;
+
+  /* This is the first part of Algorithm 2 of:
+     Rump SM, Zimmermann P, Boldo S, Melquiond G.
+     Computing predecessor and successor in rounding to nearest.
+     BIT Numer Math. 2009;49(419-431).
+     <https://doi.org/10.1007/s10543-009-0218-z>
+     The rest of Algorithm 2 is not needed because numbers less than
+     the predecessor of 1e-9 merely need to stay less than 1e-9.  */
+  double phi = DBL_EPSILON / 2 * (1 + DBL_EPSILON);
+  return c + phi * c;
+}
+
+_GL_INLINE_HEADER_END
+
+#endif
diff --git a/gl/modules/dtimespec-bound b/gl/modules/dtimespec-bound
new file mode 100644
index 000000000..736c6dabd
--- /dev/null
+++ b/gl/modules/dtimespec-bound
@@ -0,0 +1,24 @@
+Description:
+Adjust a double to provide a timespec bound.
+
+Files:
+lib/dtimespec-bound.c
+lib/dtimespec-bound.h
+
+Depends-on:
+float-h
+signbit
+
+configure.ac:
+
+Makefile.am:
+lib_SOURCES += dtimespec-bound.c
+
+Include:
+"dtimespec-bound.h"
+
+License:
+GPL
+
+Maintainer:
+all
diff --git a/src/sleep.c b/src/sleep.c
index 5871517de..13c68a606 100644
--- a/src/sleep.c
+++ b/src/sleep.c
@@ -20,10 +20,10 @@
 
 #include "system.h"
 #include "cl-strtod.h"
+#include "dtimespec-bound.h"
 #include "long-options.h"
 #include "quote.h"
 #include "xnanosleep.h"
-#include "xstrtod.h"
 
 /* The official name of this program (e.g., no 'g' prefix).  */
 #define PROGRAM_NAME "sleep"
@@ -85,7 +85,7 @@ apply_suffix (double *x, char suffix_char)
       return false;
     }
 
-  *x *= multiplier;
+  *x = dtimespec_bound (*x * multiplier, 0);
 
   return true;
 }
@@ -116,9 +116,11 @@ main (int argc, char **argv)
 
   for (int i = optind; i < argc; i++)
     {
-      double s;
-      char const *p;
-      if (! (xstrtod (argv[i], &p, &s, cl_strtod) || errno == ERANGE)
+      char *p;
+      errno = 0;
+      double duration = cl_strtod (argv[i], &p);
+      double s = dtimespec_bound (duration, errno);
+      if (argv[i] == p
           /* Nonnegative interval.  */
           || ! (0 <= s)
           /* No extra chars after the number and an optional s,m,h,d char.  */
@@ -130,7 +132,7 @@ main (int argc, char **argv)
           ok = false;
         }
 
-      seconds += s;
+      seconds = dtimespec_bound (seconds + s, 0);
     }
 
   if (!ok)
diff --git a/src/tail.c b/src/tail.c
index 1ed1c4fab..ac66c9aa5 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -35,6 +35,7 @@
 #include "assure.h"
 #include "c-ctype.h"
 #include "cl-strtod.h"
+#include "dtimespec-bound.h"
 #include "fcntl--.h"
 #include "iopoll.h"
 #include "isapipe.h"
@@ -47,7 +48,6 @@
 #include "xdectoint.h"
 #include "xnanosleep.h"
 #include "xstrtol.h"
-#include "xstrtod.h"
 
 #if HAVE_INOTIFY
 # include "hash.h"
@@ -2276,11 +2276,13 @@ parse_options (int argc, char **argv,
 
         case 's':
           {
-            double s;
-            if (! (xstrtod (optarg, nullptr, &s, cl_strtod) && 0 <= s))
+            char *ep;
+            errno = 0;
+            double s = cl_strtod (optarg, &ep);
+            if (optarg == ep || *ep || ! (0 <= s))
               error (EXIT_FAILURE, 0,
                      _("invalid number of seconds: %s"), quote (optarg));
-            *sleep_interval = s;
+            *sleep_interval = dtimespec_bound (s, errno);
           }
           break;
 
diff --git a/src/timeout.c b/src/timeout.c
index fe8eb4559..e869b2533 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -45,7 +45,6 @@
    Written by Pádraig Brady.  */
 
 #include <config.h>
-#include <ctype.h>
 #include <getopt.h>
 #include <stdio.h>
 #include <sys/types.h>
@@ -57,6 +56,7 @@
 
 #include "system.h"
 #include "cl-strtod.h"
+#include "dtimespec-bound.h"
 #include "sig2str.h"
 #include "operand2sig.h"
 #include "quote.h"
@@ -348,47 +348,32 @@ apply_time_suffix (double *x, char suffix_char)
       return false;
     }
 
-  *x *= multiplier;
+  *x = dtimespec_bound (*x * multiplier, 0);
 
   return true;
 }
 
-ATTRIBUTE_PURE static bool
-is_negative (char const *num)
-{
-  while (*num && isspace (to_uchar (*num)))
-    num++;
-  return *num == '-';
-}
-
 static double
 parse_duration (char const *str)
 {
   char *ep;
+  errno = 0;
   double duration = cl_strtod (str, &ep);
+  double s = dtimespec_bound (duration, errno);
 
   if (ep == str
       /* Nonnegative interval.  */
-      || ! (0 <= duration)
-      /* The interval did not underflow to -0.  */
-      || (errno == ERANGE && is_negative (str))
+      || ! (0 <= s)
       /* 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))
+      || !apply_time_suffix (&s, *ep))
     {
       error (0, 0, _("invalid time interval %s"), quote (str));
       usage (EXIT_CANCELED);
     }
 
-  /* 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 (duration == 0 && errno == ERANGE)
-    duration = 9.313225746154785e-10;
-
-  return duration;
+  return s;
 }
 
 static void
-- 
2.45.2