diff mbox

[v2] time: Introduce function to check correctness of nanoseconds value

Message ID 20191025132229.16703-1-lukma@denx.de
State Committed
Headers

Commit Message

Lukasz Majewski Oct. 25, 2019, 1:22 p.m. UTC 
  The valid_nanoseconds () static inline function has been introduced to
check if nanoseconds value is in the correct range - greater or equal to
zero and less than 1000000000.

The explicit #include <time.h> has been added to files where it was
missing.

Tested with:
- scripts/build-many-glibcs.py
- make PARALLELMFLAGS="-j12" && make PARALLELMFLAGS="-j12" xcheck on x86_64
---
Changes for v2:
- Use 'long int' instead of 'long' for ns
- Return single statement (instead of if)
- Use (0 <= ns && ns < 1000000000) condition instead of
  (ns >= 0 && ns <= 999999999)
---
 hurd/hurdlock.c                                | 2 +-
 hurd/hurdselect.c                              | 4 ++--
 include/time.h                                 | 8 ++++++++
 io/ppoll.c                                     | 4 ++--
 nptl/lll_timedlock_wait.c                      | 3 ++-
 nptl/pthread_cond_wait.c                       | 4 ++--
 nptl/pthread_join_common.c                     | 3 ++-
 nptl/pthread_mutex_timedlock.c                 | 4 ++--
 nptl/pthread_rwlock_common.c                   | 7 +++----
 nptl/sem_clockwait.c                           | 3 ++-
 nptl/sem_timedwait.c                           | 3 ++-
 sysdeps/htl/pt-cond-timedwait.c                | 3 ++-
 sysdeps/htl/pt-mutex-timedlock.c               | 3 ++-
 sysdeps/htl/pt-rwlock-timedrdlock.c            | 3 ++-
 sysdeps/htl/pt-rwlock-timedwrlock.c            | 3 ++-
 sysdeps/htl/sem-timedwait.c                    | 3 ++-
 sysdeps/mach/hurd/htl/pt-hurd-cond-timedwait.c | 3 ++-
 sysdeps/mach/nanosleep.c                       | 3 +--
 sysdeps/pthread/timer_settime.c                | 6 ++----
 sysdeps/sparc/sparc32/lowlevellock.c           | 3 ++-
 sysdeps/unix/clock_nanosleep.c                 | 3 +--
 sysdeps/unix/clock_settime.c                   | 2 +-
 sysdeps/unix/sysv/linux/clock_settime.c        | 2 +-
 time/clock_nanosleep.c                         | 3 +--
 24 files changed, 49 insertions(+), 36 deletions(-)

Comments

Joseph Myers Oct. 25, 2019, 9:25 p.m. UTC | #1 
On Fri, 25 Oct 2019, Lukasz Majewski wrote:

> diff --git a/include/time.h b/include/time.h
> index d93b16a781..68c6cc46cc 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -236,5 +236,13 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64)
>  
>    return tv;
>  }
> +
> +/* Check if a value is in the valid nanoseconds range. Return true if
> +   it is, false otherwise.  */
> +static inline bool
> +valid_nanoseconds (long int ns)

Since we do, unfortunately, have x32 where tv_nsec is __syscall_slong_t 
which is different from long int, I think __syscall_slong_t is what should 
be used here to keep the checks consistent with how tv_nsec is declared.  
If in future bug 16437 is fixed to make x32 use long int here (possibly by 
relying on 5.1.5 or later kernels, if the fix there to handling of 
timespec padding for compat tasks also applied to x32), this function 
should be changed to use long int again at the same time.

The patch is OK with the change to use __syscall_slong_t.
diff mbox

Patch

diff --git a/hurd/hurdlock.c b/hurd/hurdlock.c
index 0787c19661..f8aa293e61 100644
--- a/hurd/hurdlock.c
+++ b/hurd/hurdlock.c
@@ -71,7 +71,7 @@  __lll_abstimed_lock (void *ptr,
     {
       if (atomic_exchange_acq ((int *)ptr, 2) == 0)
         return 0;
-      else if (tsp->tv_nsec < 0 || tsp->tv_nsec >= 1000000000)
+      else if (! valid_nanoseconds (tsp->tv_nsec))
         return EINVAL;
 
       int mlsec = compute_reltime (tsp, clk);
diff --git a/hurd/hurdselect.c b/hurd/hurdselect.c
index 333a909d67..79cd20b03e 100644
--- a/hurd/hurdselect.c
+++ b/hurd/hurdselect.c
@@ -27,6 +27,7 @@ 
 #include <assert.h>
 #include <stdint.h>
 #include <limits.h>
+#include <time.h>
 
 /* All user select types.  */
 #define SELECT_ALL (SELECT_READ | SELECT_WRITE | SELECT_URG)
@@ -89,8 +90,7 @@  _hurd_select (int nfds,
     {
       struct timeval now;
 
-      if (timeout->tv_sec < 0 || timeout->tv_nsec < 0 ||
-	  timeout->tv_nsec >= 1000000000)
+      if (timeout->tv_sec < 0 || ! valid_nanoseconds (timeout->tv_nsec))
 	{
 	  errno = EINVAL;
 	  return -1;
diff --git a/include/time.h b/include/time.h
index d93b16a781..68c6cc46cc 100644
--- a/include/time.h
+++ b/include/time.h
@@ -236,5 +236,13 @@  valid_timespec64_to_timeval (const struct __timespec64 ts64)
 
   return tv;
 }
+
+/* Check if a value is in the valid nanoseconds range. Return true if
+   it is, false otherwise.  */
+static inline bool
+valid_nanoseconds (long int ns)
+{
+  return __glibc_likely (0 <= ns && ns < 1000000000);
+}
 #endif
 #endif
diff --git a/io/ppoll.c b/io/ppoll.c
index a387924eeb..6d156a8a46 100644
--- a/io/ppoll.c
+++ b/io/ppoll.c
@@ -22,6 +22,7 @@ 
 #include <stddef.h>	/* For NULL.  */
 #include <sys/poll.h>
 #include <sysdep-cancel.h>
+#include <time.h>
 
 
 int
@@ -33,8 +34,7 @@  ppoll (struct pollfd *fds, nfds_t nfds, const struct timespec *timeout,
   /* poll uses a simple millisecond value.  Convert it.  */
   if (timeout != NULL)
     {
-      if (timeout->tv_sec < 0
-	  || timeout->tv_nsec < 0 || timeout->tv_nsec >= 1000000000)
+      if (timeout->tv_sec < 0 || ! valid_nanoseconds (timeout->tv_nsec))
 	{
 	  __set_errno (EINVAL);
 	  return -1;
diff --git a/nptl/lll_timedlock_wait.c b/nptl/lll_timedlock_wait.c
index 03060e874b..cd3cc3d371 100644
--- a/nptl/lll_timedlock_wait.c
+++ b/nptl/lll_timedlock_wait.c
@@ -21,6 +21,7 @@ 
 #include <errno.h>
 #include <lowlevellock.h>
 #include <sys/time.h>
+#include <time.h>
 
 
 int
@@ -28,7 +29,7 @@  __lll_clocklock_wait (int *futex, clockid_t clockid,
 		      const struct timespec *abstime, int private)
 {
   /* Reject invalid timeouts.  */
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   /* Try locking.  */
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index bacae09c02..cf372bc017 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -645,7 +645,7 @@  __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
 {
   /* Check parameter validity.  This should also tell the compiler that
      it can assume that abstime is not NULL.  */
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   /* Relaxed MO is suffice because clock ID bit is only modified
@@ -668,7 +668,7 @@  __pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
 {
   /* Check parameter validity.  This should also tell the compiler that
      it can assume that abstime is not NULL.  */
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   if (!futex_abstimed_supported_clockid (clockid))
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index 9545ae4bd3..8b55c380e9 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -19,6 +19,7 @@ 
 #include "pthreadP.h"
 #include <atomic.h>
 #include <stap-probe.h>
+#include <time.h>
 
 static void
 cleanup (void *arg)
@@ -40,7 +41,7 @@  timedwait_tid (pid_t *tidp, const struct timespec *abstime)
 {
   pid_t tid;
 
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   /* Repeat until thread terminated.  */
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index a0ce044dd4..c9bb3b9176 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -235,7 +235,7 @@  __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 	    }
 
 	  /* We are about to block; check whether the timeout is invalid.  */
-	  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+	  if (! valid_nanoseconds (abstime->tv_nsec))
 	    return EINVAL;
 	  /* Work around the fact that the kernel rejects negative timeout
 	     values despite them being valid.  */
@@ -561,7 +561,7 @@  __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 		if (oldval != ceilval)
 		  {
 		    /* Reject invalid timeouts.  */
-		    if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+		    if (! valid_nanoseconds (abstime->tv_nsec))
 		      {
 			result = EINVAL;
 			goto failpp;
diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
index 7070b9c2c8..9c05e03a09 100644
--- a/nptl/pthread_rwlock_common.c
+++ b/nptl/pthread_rwlock_common.c
@@ -24,6 +24,7 @@ 
 #include <stap-probe.h>
 #include <atomic.h>
 #include <futex-internal.h>
+#include <time.h>
 
 
 /* A reader--writer lock that fulfills the POSIX requirements (but operations
@@ -290,8 +291,7 @@  __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
      if the lock can be immediately acquired" (i.e., we need not but may check
      it).  */
   if (abstime && __glibc_unlikely (!futex_abstimed_supported_clockid (clockid)
-      || abstime->tv_nsec >= 1000000000
-      || abstime->tv_nsec < 0))
+      || ! valid_nanoseconds (abstime->tv_nsec)))
     return EINVAL;
 
   /* Make sure we are not holding the rwlock as a writer.  This is a deadlock
@@ -596,8 +596,7 @@  __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
      if the lock can be immediately acquired" (i.e., we need not but may check
      it).  */
   if (abstime && __glibc_unlikely (!futex_abstimed_supported_clockid (clockid)
-      || abstime->tv_nsec >= 1000000000
-      || abstime->tv_nsec < 0))
+      || ! valid_nanoseconds (abstime->tv_nsec)))
     return EINVAL;
 
   /* Make sure we are not holding the rwlock as a writer.  This is a deadlock
diff --git a/nptl/sem_clockwait.c b/nptl/sem_clockwait.c
index 9ed98c4cce..21628df524 100644
--- a/nptl/sem_clockwait.c
+++ b/nptl/sem_clockwait.c
@@ -18,6 +18,7 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <time.h>
 #include "sem_waitcommon.c"
 
 int
@@ -32,7 +33,7 @@  sem_clockwait (sem_t *sem, clockid_t clockid,
       return -1;
     }
 
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (abstime->tv_nsec))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/nptl/sem_timedwait.c b/nptl/sem_timedwait.c
index fbb50a5fc8..a3fbe8998b 100644
--- a/nptl/sem_timedwait.c
+++ b/nptl/sem_timedwait.c
@@ -17,6 +17,7 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <time.h>
 #include "sem_waitcommon.c"
 
 /* This is in a separate file because because sem_timedwait is only provided
@@ -24,7 +25,7 @@ 
 int
 sem_timedwait (sem_t *sem, const struct timespec *abstime)
 {
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (abstime->tv_nsec))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/sysdeps/htl/pt-cond-timedwait.c b/sysdeps/htl/pt-cond-timedwait.c
index cc5a9db37f..ff5631f7ab 100644
--- a/sysdeps/htl/pt-cond-timedwait.c
+++ b/sysdeps/htl/pt-cond-timedwait.c
@@ -20,6 +20,7 @@ 
 
 #include <pt-internal.h>
 #include <pthreadP.h>
+#include <time.h>
 
 extern int __pthread_cond_timedwait_internal (pthread_cond_t *cond,
 					      pthread_mutex_t *mutex,
@@ -74,7 +75,7 @@  __pthread_cond_timedwait_internal (pthread_cond_t *cond,
   int cancelled, oldtype, drain;
   clockid_t clock_id = __pthread_default_condattr.__clock;
 
-  if (abstime && (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000))
+  if (abstime && ! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   struct __pthread *self = _pthread_self ();
diff --git a/sysdeps/htl/pt-mutex-timedlock.c b/sysdeps/htl/pt-mutex-timedlock.c
index c8fe022613..c378557f49 100644
--- a/sysdeps/htl/pt-mutex-timedlock.c
+++ b/sysdeps/htl/pt-mutex-timedlock.c
@@ -18,6 +18,7 @@ 
 
 #include <pthread.h>
 #include <assert.h>
+#include <time.h>
 
 #include <pt-internal.h>
 
@@ -119,7 +120,7 @@  __pthread_mutex_timedlock_internal (struct __pthread_mutex *mutex,
 #endif
     assert (mutex->__owner);
 
-  if (abstime != NULL && (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000))
+  if (abstime != NULL && ! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   /* Add ourselves to the queue.  */
diff --git a/sysdeps/htl/pt-rwlock-timedrdlock.c b/sysdeps/htl/pt-rwlock-timedrdlock.c
index cdae8dfd3a..d64c7a7390 100644
--- a/sysdeps/htl/pt-rwlock-timedrdlock.c
+++ b/sysdeps/htl/pt-rwlock-timedrdlock.c
@@ -18,6 +18,7 @@ 
 
 #include <pthread.h>
 #include <assert.h>
+#include <time.h>
 
 #include <pt-internal.h>
 
@@ -60,7 +61,7 @@  __pthread_rwlock_timedrdlock_internal (struct __pthread_rwlock *rwlock,
   /* Better be blocked by a writer.  */
   assert (rwlock->__readers == 0);
 
-  if (abstime != NULL && (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000))
+  if (abstime != NULL && ! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   self = _pthread_self ();
diff --git a/sysdeps/htl/pt-rwlock-timedwrlock.c b/sysdeps/htl/pt-rwlock-timedwrlock.c
index 83a022afed..2316321d79 100644
--- a/sysdeps/htl/pt-rwlock-timedwrlock.c
+++ b/sysdeps/htl/pt-rwlock-timedwrlock.c
@@ -18,6 +18,7 @@ 
 
 #include <pthread.h>
 #include <assert.h>
+#include <time.h>
 
 #include <pt-internal.h>
 
@@ -46,7 +47,7 @@  __pthread_rwlock_timedwrlock_internal (struct __pthread_rwlock *rwlock,
 
   /* The lock is busy.  */
 
-  if (abstime != NULL && (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000))
+  if (abstime != NULL && ! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   self = _pthread_self ();
diff --git a/sysdeps/htl/sem-timedwait.c b/sysdeps/htl/sem-timedwait.c
index e2eeff4b8a..dfb0cb99d3 100644
--- a/sysdeps/htl/sem-timedwait.c
+++ b/sysdeps/htl/sem-timedwait.c
@@ -19,6 +19,7 @@ 
 #include <semaphore.h>
 #include <errno.h>
 #include <assert.h>
+#include <time.h>
 
 #include <pt-internal.h>
 
@@ -39,7 +40,7 @@  __sem_timedwait_internal (sem_t *restrict sem,
       return 0;
     }
 
-  if (timeout != NULL && (timeout->tv_nsec < 0 || timeout->tv_nsec >= 1000000000))
+  if (timeout != NULL && ! valid_nanoseconds (timeout->tv_nsec))
     {
       errno = EINVAL;
       return -1;
diff --git a/sysdeps/mach/hurd/htl/pt-hurd-cond-timedwait.c b/sysdeps/mach/hurd/htl/pt-hurd-cond-timedwait.c
index cf0de47bb5..aa3eb9884c 100644
--- a/sysdeps/mach/hurd/htl/pt-hurd-cond-timedwait.c
+++ b/sysdeps/mach/hurd/htl/pt-hurd-cond-timedwait.c
@@ -19,6 +19,7 @@ 
 #include <pthread.h>
 #include <assert.h>
 #include <hurd/signal.h>
+#include <time.h>
 
 #include <pt-internal.h>
 
@@ -69,7 +70,7 @@  __pthread_hurd_cond_timedwait_internal (pthread_cond_t *cond,
 
   assert (ss->intr_port == MACH_PORT_NULL);	/* Sanity check for signal bugs. */
 
-  if (abstime != NULL && (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000))
+  if (abstime != NULL && ! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   /* Atomically enqueue our thread on the condition variable's queue of
diff --git a/sysdeps/mach/nanosleep.c b/sysdeps/mach/nanosleep.c
index 67caa3ea8a..555251f842 100644
--- a/sysdeps/mach/nanosleep.c
+++ b/sysdeps/mach/nanosleep.c
@@ -30,8 +30,7 @@  __libc_nanosleep (const struct timespec *requested_time,
   struct timeval before, after;
 
   if (requested_time->tv_sec < 0
-      || requested_time->tv_nsec < 0
-      || requested_time->tv_nsec >= 1000000000)
+      || ! valid_nanoseconds (requested_time->tv_nsec))
     {
       errno = EINVAL;
       return -1;
diff --git a/sysdeps/pthread/timer_settime.c b/sysdeps/pthread/timer_settime.c
index 3d20495283..0fad4b1983 100644
--- a/sysdeps/pthread/timer_settime.c
+++ b/sysdeps/pthread/timer_settime.c
@@ -41,10 +41,8 @@  timer_settime (timer_t timerid, int flags, const struct itimerspec *value,
       goto bail;
     }
 
-  if (value->it_interval.tv_nsec < 0
-      || value->it_interval.tv_nsec >= 1000000000
-      || value->it_value.tv_nsec < 0
-      || value->it_value.tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (value->it_interval.tv_nsec)
+      || ! valid_nanoseconds (value->it_value.tv_nsec))
     {
       __set_errno (EINVAL);
       goto bail;
diff --git a/sysdeps/sparc/sparc32/lowlevellock.c b/sysdeps/sparc/sparc32/lowlevellock.c
index 25f3e6b1a0..074ecf0636 100644
--- a/sysdeps/sparc/sparc32/lowlevellock.c
+++ b/sysdeps/sparc/sparc32/lowlevellock.c
@@ -21,6 +21,7 @@ 
 #include <sysdep.h>
 #include <lowlevellock.h>
 #include <sys/time.h>
+#include <time.h>
 
 
 void
@@ -56,7 +57,7 @@  __lll_clocklock_wait (int *futex, clockid_t clockid,
                       const struct timespec *abstime, int private)
 {
   /* Reject invalid timeouts.  */
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (abstime->tv_nsec))
     return EINVAL;
 
   do
diff --git a/sysdeps/unix/clock_nanosleep.c b/sysdeps/unix/clock_nanosleep.c
index 95b4956b12..8514a439ee 100644
--- a/sysdeps/unix/clock_nanosleep.c
+++ b/sysdeps/unix/clock_nanosleep.c
@@ -30,8 +30,7 @@  __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
 {
   struct timespec now;
 
-  if (__builtin_expect (req->tv_nsec, 0) < 0
-      || __builtin_expect (req->tv_nsec, 0) >= 1000000000)
+  if (! valid_nanoseconds (req->tv_nsec))
     return EINVAL;
 
   if (clock_id == CLOCK_THREAD_CPUTIME_ID)
diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/unix/clock_settime.c
index 18d7c99864..54c917949f 100644
--- a/sysdeps/unix/clock_settime.c
+++ b/sysdeps/unix/clock_settime.c
@@ -27,7 +27,7 @@  __clock_settime (clockid_t clock_id, const struct timespec *tp)
   int retval = -1;
 
   /* Make sure the time cvalue is OK.  */
-  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (tp->tv_nsec))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
index 54999d3008..bda113809b 100644
--- a/sysdeps/unix/sysv/linux/clock_settime.c
+++ b/sysdeps/unix/sysv/linux/clock_settime.c
@@ -26,7 +26,7 @@  int
 __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
 {
   /* Make sure the time cvalue is OK.  */
-  if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
+  if (! valid_nanoseconds (tp->tv_nsec))
     {
       __set_errno (EINVAL);
       return -1;
diff --git a/time/clock_nanosleep.c b/time/clock_nanosleep.c
index f3230c4c90..5952d29195 100644
--- a/time/clock_nanosleep.c
+++ b/time/clock_nanosleep.c
@@ -24,8 +24,7 @@  int
 __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
 		   struct timespec *rem)
 {
-  if (__builtin_expect (req->tv_nsec, 0) < 0
-      || __builtin_expect (req->tv_nsec, 0) >= 1000000000)
+  if (! valid_nanoseconds (req->tv_nsec))
     return EINVAL;
 
   if (flags != TIMER_ABSTIME && flags != 0)