bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] timespec: fix resolution confusion


From: Paul Eggert
Subject: [PATCH] timespec: fix resolution confusion
Date: Mon, 10 Sep 2018 18:44:02 -0700

In normal usage, clock resolution is given in seconds, but the
code was mistakenly using inverse seconds and calling it
“resolution”.  Fix this, partly by renaming two identifiers.
The old names will be kept for a bit, to ease transition.
* lib/timespec.h (TIMESPEC_HZ, LOG10_TIMESPEC_HZ):
New constants, replacing TIMESPEC_RESOLUTION and
LOG10_TIMESPEC_RESOLUTION, which are now obsolescent.
All uses changed.
---
 ChangeLog                          | 12 ++++++++++++
 doc/posix-functions/futimens.texi  |  2 +-
 doc/posix-functions/utimensat.texi |  2 +-
 lib/dtotimespec.c                  | 10 +++++-----
 lib/stat-time.h                    |  8 ++++----
 lib/timespec-add.c                 |  6 +++---
 lib/timespec-sub.c                 |  6 +++---
 lib/timespec.h                     | 18 ++++++++++++------
 lib/utimens.c                      |  4 ++--
 lib/utimensat.c                    |  4 ++--
 tests/test-futimens.h              |  2 +-
 tests/test-timespec.c              | 30 +++++++++++++++---------------
 tests/test-utimens.h               |  2 +-
 13 files changed, 62 insertions(+), 44 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b8e7ec905..baf02fd16 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2018-09-10  Paul Eggert  <address@hidden>
+
+       timespec: fix resolution confusion
+       In normal usage, clock resolution is given in seconds, but the
+       code was mistakenly using inverse seconds and calling it
+       “resolution”.  Fix this, partly by renaming two identifiers.
+       The old names will be kept for a bit, to ease transition.
+       * lib/timespec.h (TIMESPEC_HZ, LOG10_TIMESPEC_HZ):
+       New constants, replacing TIMESPEC_RESOLUTION and
+       LOG10_TIMESPEC_RESOLUTION, which are now obsolescent.
+       All uses changed.
+
 2018-09-09  Paul Eggert  <address@hidden>
 
        mktime: simplify in prep for glibc merge
diff --git a/doc/posix-functions/futimens.texi 
b/doc/posix-functions/futimens.texi
index 7d1687eb7..19fb84ea6 100644
--- a/doc/posix-functions/futimens.texi
+++ b/doc/posix-functions/futimens.texi
@@ -12,7 +12,7 @@ Portability problems fixed by Gnulib:
 This function is missing on some platforms:
 glibc 2.3.6, Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8, 
AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 10, Cygwin 1.5.x, mingw, MSVC 
14, Interix 3.5, BeOS, Android 4.3.
 However, the replacement function may end up truncating timestamps to
-less resolution than supported by the file system.
+worse resolution than supported by the file system.
 @item
 This function returns a bogus value instead of failing with
 @code{ENOSYS} on some platforms:
diff --git a/doc/posix-functions/utimensat.texi 
b/doc/posix-functions/utimensat.texi
index d0590bb01..9182fd6ae 100644
--- a/doc/posix-functions/utimensat.texi
+++ b/doc/posix-functions/utimensat.texi
@@ -12,7 +12,7 @@ Portability problems fixed by Gnulib:
 This function is missing on some platforms:
 glibc 2.3.6, Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8, 
AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 10, Cygwin 1.5.x, mingw, MSVC 
14, Interix 3.5, BeOS, Android 3.0.
 However, the replacement function may end up truncating timestamps to
-less resolution than supported by the file system.  Furthermore, the
+worse resolution than supported by the file system.  Furthermore, the
 replacement function is not safe to be used in libraries and is not
 multithread-safe.
 @item
diff --git a/lib/dtotimespec.c b/lib/dtotimespec.c
index 599f7427a..dcbd28051 100644
--- a/lib/dtotimespec.c
+++ b/lib/dtotimespec.c
@@ -32,20 +32,20 @@ dtotimespec (double sec)
   if (! (TYPE_MINIMUM (time_t) < sec))
     return make_timespec (TYPE_MINIMUM (time_t), 0);
   else if (! (sec < 1.0 + TYPE_MAXIMUM (time_t)))
-    return make_timespec (TYPE_MAXIMUM (time_t), TIMESPEC_RESOLUTION - 1);
+    return make_timespec (TYPE_MAXIMUM (time_t), TIMESPEC_HZ - 1);
   else
     {
       time_t s = sec;
-      double frac = TIMESPEC_RESOLUTION * (sec - s);
+      double frac = TIMESPEC_HZ * (sec - s);
       long ns = frac;
       ns += ns < frac;
-      s += ns / TIMESPEC_RESOLUTION;
-      ns %= TIMESPEC_RESOLUTION;
+      s += ns / TIMESPEC_HZ;
+      ns %= TIMESPEC_HZ;
 
       if (ns < 0)
         {
           s--;
-          ns += TIMESPEC_RESOLUTION;
+          ns += TIMESPEC_HZ;
         }
 
       return make_timespec (s, ns);
diff --git a/lib/stat-time.h b/lib/stat-time.h
index 8e787bd3b..69ebe85df 100644
--- a/lib/stat-time.h
+++ b/lib/stat-time.h
@@ -213,7 +213,7 @@ stat_time_normalize (int result, struct stat *st _GL_UNUSED)
 #if defined __sun && defined STAT_TIMESPEC
   if (result == 0)
     {
-      long int timespec_resolution = 1000000000;
+      long int timespec_hz = 1000000000;
       short int const ts_off[] = { offsetof (struct stat, st_atim),
                                    offsetof (struct stat, st_mtim),
                                    offsetof (struct stat, st_ctim) };
@@ -221,11 +221,11 @@ stat_time_normalize (int result, struct stat *st 
_GL_UNUSED)
       for (i = 0; i < sizeof ts_off / sizeof *ts_off; i++)
         {
           struct timespec *ts = (struct timespec *) ((char *) st + ts_off[i]);
-          long int q = ts->tv_nsec / timespec_resolution;
-          long int r = ts->tv_nsec % timespec_resolution;
+          long int q = ts->tv_nsec / timespec_hz;
+          long int r = ts->tv_nsec % timespec_hz;
           if (r < 0)
             {
-              r += timespec_resolution;
+              r += timespec_hz;
               q--;
             }
           ts->tv_nsec = r;
diff --git a/lib/timespec-add.c b/lib/timespec-add.c
index f6a8c38b3..1913b979e 100644
--- a/lib/timespec-add.c
+++ b/lib/timespec-add.c
@@ -18,7 +18,7 @@
 /* Written by Paul Eggert.  */
 
 /* Return the sum of two timespec values A and B.  On overflow, return
-   an extremal value.  This assumes 0 <= tv_nsec < TIMESPEC_RESOLUTION.  */
+   an extremal value.  This assumes 0 <= tv_nsec < TIMESPEC_HZ.  */
 
 #include <config.h>
 #include "timespec.h"
@@ -31,7 +31,7 @@ timespec_add (struct timespec a, struct timespec b)
   time_t rs = a.tv_sec;
   time_t bs = b.tv_sec;
   int ns = a.tv_nsec + b.tv_nsec;
-  int nsd = ns - TIMESPEC_RESOLUTION;
+  int nsd = ns - TIMESPEC_HZ;
   int rns = ns;
   time_t tmin = TYPE_MINIMUM (time_t);
   time_t tmax = TYPE_MAXIMUM (time_t);
@@ -63,7 +63,7 @@ timespec_add (struct timespec a, struct timespec b)
         {
         high_overflow:
           rs = tmax;
-          rns = TIMESPEC_RESOLUTION - 1;
+          rns = TIMESPEC_HZ - 1;
         }
     }
 
diff --git a/lib/timespec-sub.c b/lib/timespec-sub.c
index 398a6a5de..9eac36e51 100644
--- a/lib/timespec-sub.c
+++ b/lib/timespec-sub.c
@@ -19,7 +19,7 @@
 
 /* Return the difference between two timespec values A and B.  On
    overflow, return an extremal value.  This assumes 0 <= tv_nsec <
-   TIMESPEC_RESOLUTION.  */
+   TIMESPEC_HZ.  */
 
 #include <config.h>
 #include "timespec.h"
@@ -38,7 +38,7 @@ timespec_sub (struct timespec a, struct timespec b)
 
   if (ns < 0)
     {
-      rns = ns + TIMESPEC_RESOLUTION;
+      rns = ns + TIMESPEC_HZ;
       if (bs < tmax)
         bs++;
       else if (- TYPE_SIGNED (time_t) < rs)
@@ -63,7 +63,7 @@ timespec_sub (struct timespec a, struct timespec b)
       else
         {
           rs = tmax;
-          rns = TIMESPEC_RESOLUTION - 1;
+          rns = TIMESPEC_HZ - 1;
         }
     }
 
diff --git a/lib/timespec.h b/lib/timespec.h
index 94ba8d0b6..c414cfe45 100644
--- a/lib/timespec.h
+++ b/lib/timespec.h
@@ -35,11 +35,17 @@ extern "C" {
 
 #include "verify.h"
 
-/* Resolution of timespec timestamps (in units per second), and log
-   base 10 of the resolution.  */
+/* Inverse resolution of timespec timestamps (in units per second),
+   and log base 10 of the inverse resolution.  */
 
-enum { TIMESPEC_RESOLUTION = 1000000000 };
-enum { LOG10_TIMESPEC_RESOLUTION = 9 };
+enum { TIMESPEC_HZ = 1000000000 };
+enum { LOG10_TIMESPEC_HZ = 9 };
+
+/* Obsolescent names for backward compatibility.
+   They are misnomers, because TIMESPEC_RESOLUTION is not a resolution.  */
+
+enum { TIMESPEC_RESOLUTION = TIMESPEC_HZ };
+enum { LOG10_TIMESPEC_RESOLUTION = LOG10_TIMESPEC_HZ };
 
 /* Return a timespec with seconds S and nanoseconds NS.  */
 
@@ -88,8 +94,8 @@ timespec_cmp (struct timespec a, struct timespec b)
 
   /* Pacify gcc -Wstrict-overflow (bleeding-edge circa 2017-10-02).  See:
      https://lists.gnu.org/r/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);
+  assume (-1 <= a.tv_nsec && a.tv_nsec <= 2 * TIMESPEC_HZ);
+  assume (-1 <= b.tv_nsec && b.tv_nsec <= 2 * TIMESPEC_HZ);
 
   return a.tv_nsec - b.tv_nsec;
 }
diff --git a/lib/utimens.c b/lib/utimens.c
index e65f55d82..f6c4fe34c 100644
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -91,11 +91,11 @@ validate_timespec (struct timespec timespec[2])
   if ((timespec[0].tv_nsec != UTIME_NOW
        && timespec[0].tv_nsec != UTIME_OMIT
        && ! (0 <= timespec[0].tv_nsec
-             && timespec[0].tv_nsec < TIMESPEC_RESOLUTION))
+             && timespec[0].tv_nsec < TIMESPEC_HZ))
       || (timespec[1].tv_nsec != UTIME_NOW
           && timespec[1].tv_nsec != UTIME_OMIT
           && ! (0 <= timespec[1].tv_nsec
-                && timespec[1].tv_nsec < TIMESPEC_RESOLUTION)))
+                && timespec[1].tv_nsec < TIMESPEC_HZ)))
     {
       errno = EINVAL;
       return -1;
diff --git a/lib/utimensat.c b/lib/utimensat.c
index 72ac1b4ad..6d4144cb4 100644
--- a/lib/utimensat.c
+++ b/lib/utimensat.c
@@ -94,10 +94,10 @@ rpl_utimensat (int fd, char const *file, struct timespec 
const times[2],
       else if (times
                && ((times[0].tv_nsec != UTIME_NOW
                     && ! (0 <= times[0].tv_nsec
-                          && times[0].tv_nsec < TIMESPEC_RESOLUTION))
+                          && times[0].tv_nsec < TIMESPEC_HZ))
                    || (times[1].tv_nsec != UTIME_NOW
                        && ! (0 <= times[1].tv_nsec
-                             && times[1].tv_nsec < TIMESPEC_RESOLUTION))))
+                             && times[1].tv_nsec < TIMESPEC_HZ))))
         {
           errno = EINVAL;
           return -1;
diff --git a/tests/test-futimens.h b/tests/test-futimens.h
index 0c5db1fdd..47c4d888a 100644
--- a/tests/test-futimens.h
+++ b/tests/test-futimens.h
@@ -47,7 +47,7 @@ test_futimens (int (*func) (int, struct timespec const *),
     }
   ASSERT (!result);
   ASSERT (fstat (fd, &st2) == 0);
-  /* If utimens truncates to less resolution than the file system
+  /* If utimens truncates to worse resolution than the file system
      supports, then time can appear to go backwards between now and a
      follow-up utimens with UTIME_NOW or a NULL timespec.  Use
      UTIMECMP_TRUNCATE_SOURCE to compensate, with st1 as the
diff --git a/tests/test-timespec.c b/tests/test-timespec.c
index 4acc1aee4..2abeac5e8 100644
--- a/tests/test-timespec.c
+++ b/tests/test-timespec.c
@@ -30,36 +30,36 @@ static struct { int s; int ns; } const prototype[] =
   {
     { INT_MIN, 0 },
     { INT_MIN, 1 },
-    { INT_MIN, TIMESPEC_RESOLUTION - 1 },
+    { INT_MIN, TIMESPEC_HZ - 1 },
     { INT_MIN + 1, 0 },
     { INT_MIN + 1, 1 },
-    { INT_MIN + 1, TIMESPEC_RESOLUTION - 1 },
+    { INT_MIN + 1, TIMESPEC_HZ - 1 },
     { -1, 0 },
     { -1, 1 },
-    { -1, TIMESPEC_RESOLUTION - 1 },
+    { -1, TIMESPEC_HZ - 1 },
     { 0, 0 },
     { 0, 1 },
-    { 0, TIMESPEC_RESOLUTION - 1 },
+    { 0, TIMESPEC_HZ - 1 },
     { 1, 0 },
     { 1, 1 },
-    { 1, TIMESPEC_RESOLUTION - 1 },
+    { 1, TIMESPEC_HZ - 1 },
     { 1234567890, 0 },
     { 1234567890, 1 },
-    { 1234567890, TIMESPEC_RESOLUTION - 1 },
+    { 1234567890, TIMESPEC_HZ - 1 },
     { INT_MAX - 1, 0 },
     { INT_MAX - 1, 1 },
-    { INT_MAX - 1, TIMESPEC_RESOLUTION - 1 },
+    { INT_MAX - 1, TIMESPEC_HZ - 1 },
     { INT_MAX, 0 },
     { INT_MAX, 1 },
-    { INT_MAX, TIMESPEC_RESOLUTION - 1 },
-    { INT_MAX, 2 * TIMESPEC_RESOLUTION }
+    { INT_MAX, TIMESPEC_HZ - 1 },
+    { INT_MAX, 2 * TIMESPEC_HZ }
   };
 enum { nprototypes = sizeof prototype / sizeof *prototype };
 
 static bool
 valid (struct timespec a)
 {
-  return 0 <= a.tv_nsec && a.tv_nsec < TIMESPEC_RESOLUTION;
+  return 0 <= a.tv_nsec && a.tv_nsec < TIMESPEC_HZ;
 }
 
 static int
@@ -85,7 +85,7 @@ extremal (struct timespec a)
 {
   return ((a.tv_sec == TYPE_MINIMUM (time_t) && a.tv_nsec == 0)
           || (a.tv_sec == TYPE_MAXIMUM (time_t)
-              && a.tv_nsec == TIMESPEC_RESOLUTION - 1));
+              && a.tv_nsec == TIMESPEC_HZ - 1));
 }
 
 int
@@ -94,7 +94,7 @@ main (void)
   int i, j, k;
   struct timespec test[nprototypes + 1];
   int ntests;
-  int computed_resolution = 1;
+  int computed_hz = 1;
   struct timespec prevroundtrip;
 
   test[0] = make_timespec (TYPE_MINIMUM (time_t), -1);
@@ -111,9 +111,9 @@ main (void)
         }
     }
 
-  for (i = 0; i < LOG10_TIMESPEC_RESOLUTION; i++)
-    computed_resolution *= 10;
-  ASSERT (computed_resolution == TIMESPEC_RESOLUTION);
+  for (i = 0; i < LOG10_TIMESPEC_HZ; i++)
+    computed_hz *= 10;
+  ASSERT (computed_hz == TIMESPEC_HZ);
 
   for (i = 0; i < ntests; i++)
     {
diff --git a/tests/test-utimens.h b/tests/test-utimens.h
index 06f242089..f5984065a 100644
--- a/tests/test-utimens.h
+++ b/tests/test-utimens.h
@@ -27,7 +27,7 @@ test_utimens (int (*func) (char const *, struct timespec 
const *), bool print)
   struct stat st2;
 
   ASSERT (close (creat (BASE "file", 0600)) == 0);
-  /* If utimens truncates to less resolution than the file system
+  /* If utimens truncates to worse resolution than the file system
      supports, then time can appear to go backwards between now and a
      follow-up utimens with UTIME_NOW or a NULL timespec.  Use
      UTIMECMP_TRUNCATE_SOURCE to compensate, with st1 as the
-- 
2.17.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]