Re: [PATCH] timespec: fill in other members

[Bruno Haible]

Paul Eggert wrote:
> @@ -143,8 +142,7 @@ gettimeofday (struct timeval *restrict tv, void *restrict 
> tz)
>  #   error "Only 1-second nominal clock resolution found.  Is that intended?" 
> \
>            "If so, compile with the -DOK_TO_USE_1S_CLOCK option."
>  #  endif
> -  tv->tv_sec = time (NULL);
> -  tv->tv_usec = 0;
> +  *tv = (struct timeval) { .tv_sec = time (NULL) };
>  
>    return 0;
>  

This coding style hampers maintainability: The reader will wonder what's the
intent of this code. "Why is it initializing tv_sec but not tv_usec?" IMO,
if the code intends to initialize tv_usec, it should do so explicitly.
If a field that a programmer who reads the code is relevant (in the sense
that the programmer thinks about it), it should be listed.
Even if the reader knows that an omitted field is zero-initialized by this
syntax, according to C99, it costs them one fewer brain cycle if the
initializer is explicit.

Omitting the initializer should IMO be limited to cases where it's irrelevant,
such as the .tm_wday field of a 'struct tm' used as input to 'mktime'.

I'm fixing this in the part of gnulib that I personally maintain.


2023-05-15  Bruno Haible  <bruno@clisp.org>

        gettimeofday, pthread-*, thread, thrd: Don't omit intended initializers.
        * lib/gettimeofday.c (gettimeofday): List the initializers of both
        tv_sec and tv_usec.
        * lib/glthread/thread.c (gl_thread_self): List the initializers of both
        tv_sec and tv_nsec.
        * lib/pthread-cond.c (pthread_cond_wait): Likewise.
        * lib/thrd.c (rpl_thrd_current): Likewise.
        * lib/pthread-rwlock.c (MIN): New macro.
        (pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock): List the
        initializers of both tv_sec and tv_nsec. Don't modify the duration after
        having initialized it.
        * lib/pthread_mutex_timedlock.c (MIN): New macro.
        (pthread_mutex_timedlock): List the initializers of both tv_sec and
        tv_nsec. Don't modify the duration after having initialized it.

diff --git a/lib/gettimeofday.c b/lib/gettimeofday.c
index 69b43a62c8..c71629cbc5 100644
--- a/lib/gettimeofday.c
+++ b/lib/gettimeofday.c
@@ -142,7 +142,7 @@ gettimeofday (struct timeval *restrict tv, void *restrict 
tz)
 #   error "Only 1-second nominal clock resolution found.  Is that intended?" \
           "If so, compile with the -DOK_TO_USE_1S_CLOCK option."
 #  endif
-  *tv = (struct timeval) { .tv_sec = time (NULL) };
+  *tv = (struct timeval) { .tv_sec = time (NULL), .tv_usec = 0 };
 
   return 0;
 
diff --git a/lib/glthread/thread.c b/lib/glthread/thread.c
index 3352159edc..ce6b9a83ed 100644
--- a/lib/glthread/thread.c
+++ b/lib/glthread/thread.c
@@ -139,7 +139,11 @@ gl_thread_self (void)
             /* Memory allocation failed.  There is not much we can do.  Have to
                busy-loop, waiting for the availability of memory.  */
             {
-              struct timespec ts = { .tv_sec = 1 };
+              struct timespec ts =
+                {
+                  .tv_sec = 1,
+                  .tv_nsec = 0
+                };
               thrd_sleep (&ts, NULL);
             }
           }
diff --git a/lib/pthread-cond.c b/lib/pthread-cond.c
index 6980cc6ed9..7c94e47b1a 100644
--- a/lib/pthread-cond.c
+++ b/lib/pthread-cond.c
@@ -115,7 +115,11 @@ pthread_cond_wait (_GL_UNUSED pthread_cond_t *cond,
      Wait endlessly.  */
   for (;;)
     {
-      struct timespec duration = { .tv_sec = 86400 };
+      struct timespec duration =
+        {
+          .tv_sec = 86400,
+          .tv_nsec = 0
+        };
       nanosleep (&duration, NULL);
     }
 }
diff --git a/lib/pthread-rwlock.c b/lib/pthread-rwlock.c
index 5087661b0b..25ff5eeb37 100644
--- a/lib/pthread-rwlock.c
+++ b/lib/pthread-rwlock.c
@@ -30,6 +30,10 @@
 # include <time.h>
 #endif
 
+#ifndef MIN
+# define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
 #if ((defined _WIN32 && ! defined __CYGWIN__) && USE_WINDOWS_THREADS) || 
!HAVE_PTHREAD_H
 
 int
@@ -409,9 +413,11 @@ pthread_rwlock_timedrdlock (pthread_rwlock_t *lock,
         return ETIMEDOUT;
 
       /* Sleep 1 ms.  */
-      struct timespec duration = { .tv_nsec = 1000000 };
-      if (duration.tv_nsec > remaining)
-        duration.tv_nsec = remaining;
+      struct timespec duration =
+        {
+          .tv_sec = 0,
+          .tv_nsec = MIN (1000000, remaining)
+        };
       nanosleep (&duration, NULL);
     }
 }
@@ -464,9 +470,11 @@ pthread_rwlock_timedwrlock (pthread_rwlock_t *lock,
         return ETIMEDOUT;
 
       /* Sleep 1 ms.  */
-      struct timespec duration = { .tv_nsec = 1000000 };
-      if (duration.tv_nsec > remaining)
-        duration.tv_nsec = remaining;
+      struct timespec duration =
+        {
+          .tv_sec = 0,
+          .tv_nsec = MIN (1000000, remaining)
+        };
       nanosleep (&duration, NULL);
     }
 }
diff --git a/lib/pthread_mutex_timedlock.c b/lib/pthread_mutex_timedlock.c
index 2394092e83..2e931bb1d8 100644
--- a/lib/pthread_mutex_timedlock.c
+++ b/lib/pthread_mutex_timedlock.c
@@ -24,6 +24,10 @@
 #include <sys/time.h>
 #include <time.h>
 
+#ifndef MIN
+# define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
 int
 pthread_mutex_timedlock (pthread_mutex_t *mutex, const struct timespec 
*abstime)
 {
@@ -77,9 +81,11 @@ pthread_mutex_timedlock (pthread_mutex_t *mutex, const 
struct timespec *abstime)
         return ETIMEDOUT;
 
       /* Sleep 1 ms.  */
-      struct timespec duration = { .tv_nsec = 1000000 };
-      if (duration.tv_nsec > remaining)
-        duration.tv_nsec = remaining;
+      struct timespec duration =
+        {
+          .tv_sec = 0,
+          .tv_nsec = MIN (1000000, remaining)
+        };
       nanosleep (&duration, NULL);
     }
 }
diff --git a/lib/thrd.c b/lib/thrd.c
index 1a13a51e00..5e9a988c2c 100644
--- a/lib/thrd.c
+++ b/lib/thrd.c
@@ -143,7 +143,11 @@ rpl_thrd_current (void)
             /* Memory allocation failed.  There is not much we can do.  Have to
                busy-loop, waiting for the availability of memory.  */
             {
-              struct timespec ts = { .tv_sec = 1 };
+              struct timespec ts =
+                {
+                  .tv_sec = 1,
+                  .tv_nsec = 0
+                };
               thrd_sleep (&ts, NULL);
             }
           }