Jim Meyering wrote:
Here's a proposed patch:
I prefer 'assume' to 'assure' here, since it's a low-level time-comparison
primitive and lots of other code in the module already silently assumes that the
timestamps are valid. Also, while I was in the neighborhood I noticed that the
cast is no longer needed, since the module provokes -Wconversion warnings in
several other places now (and I expect nobody notices because nobody looks at
those warnings any more). So I installed the attached followup.
From f466816e06ec3516567c3edcd0219bd1f9b736eb Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sun, 29 Oct 2017 16:22:41 -0700
Subject: [PATCH] =?UTF-8?q?timespec:=20prefer=20=E2=80=98assume=E2=80=99?=
=?UTF-8?q?=20to=20=E2=80=98assure=E2=80=99?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This avoids some runtime tests. The rest of the module makes
similar assumptions and there is little point to testing here.
* lib/timespec.h: Include verify.h instead of assure.h.
(timespec_cmp): Use ‘assume’, not ‘assure’.
Also, remove an unnecessary cast to ‘int’, as lots of other
code in this module now causes -Wconversion to complain, and
this is a problem with -Wconversion not with the code.
* modules/timespec (Depends-on): Depend on ‘verify’, not ‘assure’.
---
ChangeLog | 11 +++++++++++
lib/timespec.h | 26 ++++++++++++++------------
modules/timespec | 2 +-
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 16506ba..87a9297 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
2017-10-29 Paul Eggert <egg...@cs.ucla.edu>
+ timespec: prefer ‘assume’ to ‘assure’
+ This avoids some runtime tests. The rest of the module makes
+ similar assumptions and there is little point to testing here.
+ * lib/timespec.h: Include verify.h instead of assure.h.
+ (timespec_cmp): Use ‘assume’, not ‘assure’.
+ Also, remove an unnecessary cast to ‘int’, as lots of other
+ code in this module now causes -Wconversion to complain, and
+ this is a problem with -Wconversion not with the code.
+
+ * modules/timespec (Depends-on): Depend on ‘verify’, not ‘assure’.
+
Port recent gnulib-tool change to Dash
* gnulib-tool (func_create_testdir): Don't assume that the shell
retokenizes after expanding "$@" inside the call to
diff --git a/lib/timespec.h b/lib/timespec.h
index 61cfebb..cc34067 100644
--- a/lib/timespec.h
+++ b/lib/timespec.h
@@ -33,7 +33,7 @@ _GL_INLINE_HEADER_BEGIN
extern "C" {
#endif
-#include "assure.h"
+#include "verify.h"
/* Resolution of timespec timestamps (in units per second), and log
base 10 of the resolution. */
@@ -69,27 +69,29 @@ make_timespec (time_t s, long int ns)
any platform of interest to the GNU project, since all such
platforms have 32-bit int or wider.
- Replacing "(int) (a.tv_nsec - b.tv_nsec)" with something like
+ Replacing "a.tv_nsec - b.tv_nsec" with something like
"a.tv_nsec < b.tv_nsec ? -1 : a.tv_nsec > b.tv_nsec" would cause
this function to work in some cases where the above assumption is
violated, but not in all cases (e.g., a.tv_sec==1, a.tv_nsec==-2,
b.tv_sec==0, b.tv_nsec==999999999) and is arguably not worth the
extra instructions. Using a subtraction has the advantage of
detecting some invalid cases on platforms that detect integer
- overflow.
-
- The (int) cast avoids a gcc -Wconversion warning. */
+ overflow. */
_GL_TIMESPEC_INLINE int _GL_ATTRIBUTE_PURE
timespec_cmp (struct timespec a, struct timespec b)
{
- /* These assure calls teach gcc7 enough so that its
- -Wstrict-overflow does not complain about the following code. */
- assure (-1 <= a.tv_nsec && a.tv_nsec <= 2 * TIMESPEC_RESOLUTION);
- assure (-1 <= b.tv_nsec && b.tv_nsec <= 2 * TIMESPEC_RESOLUTION);
- return (a.tv_sec < b.tv_sec ? -1
- : a.tv_sec > b.tv_sec ? 1
- : (int) (a.tv_nsec - b.tv_nsec));
+ if (a.tv_sec < b.tv_sec)
+ return -1;
+ if (a.tv_sec > b.tv_sec)
+ return 1;
+
+ /* Pacify gcc -Wstrict-overflow (bleeding-edge circa 2017-10-02). See:
+ http://lists.gnu.org/archive/html/bug-gnulib/2017-10/msg00006.html */
+ assume (-1 <= a.tv_nsec && a.tv_nsec <= 2 * TIMESPEC_RESOLUTION);
+ assume (-1 <= b.tv_nsec && b.tv_nsec <= 2 * TIMESPEC_RESOLUTION);
+
+ return a.tv_nsec - b.tv_nsec;
}
/* Return -1, 0, 1, depending on the sign of A. A.tv_nsec must be
diff --git a/modules/timespec b/modules/timespec
index 01ab6ad..e6e1514 100644
--- a/modules/timespec
+++ b/modules/timespec
@@ -7,9 +7,9 @@ lib/timespec.c
m4/timespec.m4
Depends-on:
-assure
extern-inline
time
+verify
configure.ac:
gl_TIMESPEC
--
2.7.4
