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 <egg...@cs.ucla.edu> 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
