On 12/22/19 3:31 AM, Bruno Haible wrote:
> diff --git a/lib/xtime.h b/lib/xtime.h
> index 77f1c30..5e0ae89 100644
> --- a/lib/xtime.h
> +++ b/lib/xtime.h
> @@ -42,12 +42,13 @@ extern "C" {
> XTIME_INLINE xtime_t
> xtime_make (xtime_t s, long int ns)
> {
> - const long int giga = 1000 * 1000 * 1000;
> - s += ns / giga;
> - ns %= giga;
> return XTIME_PRECISION * s + ns;
> }
Akim put in that code in October 2018, but I can't see the need for it either. I
installed the first attached patch to revert that, and am cc'ing this to Akim to
see why it might be needed. If it is needed, we'll need to complicate the
overflow suppression a bit.
> xtime_sec (xtime_t t)
> {
> return (t < 0
> - ? (t + XTIME_PRECISION - 1) / XTIME_PRECISION - 1
> + ? (t + 1) / XTIME_PRECISION - 1
> : xtime_nonnegative_sec (t));
Thanks for pointing out the bug. We can simplify the fix further (and speed it
up a bit on typical hosts). I installed the second attached patch to do that.
>From e994d4b36b8fc37844fe39cea0f30501864e2db7 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sun, 22 Dec 2019 12:38:22 -0800
Subject: [PATCH 1/2] gethrxtime: remove incorrect overflow detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This is part of a patch written by Bruno Haible:
https://lists.gnu.org/r/bug-gnulib/2019-12/msg00192.html
* lib/xtime.h (xtime_make): Remove attempt to prevent internal
integer overflow, as it didn’t suffice. This reverts the xtime.h
part of 2018-10-12T04:46:09Z!akim.demai...@gmail.com, which I
cannot now see the need for anyway (even in cases where it works),
as the patch is helpful only when the signs of S and NS disagree,
and all callers pass nonnegative values for S and NS.
Instead, add a comment saying args should be nonnegative.
---
ChangeLog | 10 ++++++++++
lib/xtime.h | 10 ++++++----
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index b5dedb14d..25c8c4755 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2019-12-22 Bruno Haible <br...@clisp.org>
+
+ gethrxtime: remove incorrect overflow detection
+ * lib/xtime.h (xtime_make): Remove attempt to prevent internal
+ integer overflow, as it didn’t suffice. This reverts the xtime.h
+ part of 2018-10-12T04:46:09Z!akim.demai...@gmail.com, which I
+ cannot now see the need for anyway (even in cases where it works),
+ as the patch is helpful only when the signs of S and NS disagree,
+ and all callers pass nonnegative values for S and NS.
+
2019-12-22 Bruno Haible <br...@clisp.org>
setlocale-null: Add standalone include file.
diff --git a/lib/xtime.h b/lib/xtime.h
index 77f1c30fa..a442da5c0 100644
--- a/lib/xtime.h
+++ b/lib/xtime.h
@@ -38,16 +38,18 @@ extern "C" {
#endif
/* Return an extended time value that contains S seconds and NS
- nanoseconds. */
+ nanoseconds. S and NS should be nonnegative; otherwise, integer
+ overflow can occur even if the result is in range. */
XTIME_INLINE xtime_t
xtime_make (xtime_t s, long int ns)
{
- const long int giga = 1000 * 1000 * 1000;
- s += ns / giga;
- ns %= giga;
return XTIME_PRECISION * s + ns;
}
+/* The following functions split an extended time value:
+ T = XTIME_PRECISION * xtime_sec (T) + xtime_nsec (T)
+ with 0 <= xtime_nsec (T) < XTIME_PRECISION. */
+
/* Return the number of seconds in T, which must be nonnegative. */
XTIME_INLINE xtime_t
xtime_nonnegative_sec (xtime_t t)
--
2.17.1
>From a39cf946bf1db2175d65749474cca12c9e88d842 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sun, 22 Dec 2019 12:32:31 -0800
Subject: [PATCH 2/2] gethrxtime: fix rounding bug with negative args
Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2019-12/msg00192.html
* lib/xtime.h (xtime_sec): Simplify calculation and correct bug
with negative rounding. Common platforms can compute / and % with
a single instruction, so the simplified code should be shorter and
faster on these platforms anyway.
---
ChangeLog | 10 ++++++++++
lib/xtime.h | 4 +---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 25c8c4755..47cbfb2cd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2019-12-22 Paul Eggert <egg...@cs.ucla.edu>
+
+ gethrxtime: fix rounding bug with negative args
+ Problem reported by Bruno Haible in:
+ https://lists.gnu.org/r/bug-gnulib/2019-12/msg00192.html
+ * lib/xtime.h (xtime_sec): Simplify calculation and correct bug
+ with negative rounding. Common platforms can compute / and % with
+ a single instruction, so the simplified code should be shorter and
+ faster on these platforms anyway.
+
2019-12-22 Bruno Haible <br...@clisp.org>
gethrxtime: remove incorrect overflow detection
diff --git a/lib/xtime.h b/lib/xtime.h
index a442da5c0..5dea46767 100644
--- a/lib/xtime.h
+++ b/lib/xtime.h
@@ -61,9 +61,7 @@ xtime_nonnegative_sec (xtime_t t)
XTIME_INLINE xtime_t
xtime_sec (xtime_t t)
{
- return (t < 0
- ? (t + XTIME_PRECISION - 1) / XTIME_PRECISION - 1
- : xtime_nonnegative_sec (t));
+ return t / XTIME_PRECISION - (t % XTIME_PRECISION < 0);
}
/* Return the number of nanoseconds in T, which must be nonnegative. */
--
2.17.1
