nptl: pthread_rwlock: Move timeout validation into _full functions

As recommended by the comments in the implementations of
pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock, let's move
the timeout validity checks into the corresponding pthread_rwlock_rdlock_full
and pthread_rwlock_wrlock_full functions. Since these functions may be
called with abstime == NULL, an extra check for that is necessary too.

	* nptl/pthread_rwlock_common.c (__pthread_rwlock_rdlock_full):
	Check validity of abstime parameter.
	(__pthread_rwlock_rwlock_full): Likewise.
	* nptl/pthread_rwlock_timedrdlock.c
	* (pthread_rwlock_timedrdlock):
	Remove check for validity of abstime parameter.
	* nptl/pthread_rwlock_timedwrlock.c
	* (pthread_rwlock_timedwrlock):
	Likewise.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

diff --git a/ChangeLog b/ChangeLog
index 718f8f4d385fd8055045689195ab754b3f20b76a..e57fc446be0ea8308dbaf6e840ead1ef9a4045f7 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2019-07-12  Mike Crowe  <mac@mcrowe.com>
 
+       nptl: pthread_rwlock: Move timeout validation into _full functions
+       * nptl/pthread_rwlock_common.c (__pthread_rwlock_rdlock_full):
+       Check validity of abstime parameter.
+       (__pthread_rwlock_rwlock_full): Likewise.
+       * nptl/pthread_rwlock_timedrdlock.c (pthread_rwlock_timedrdlock):
+       Remove check for validity of abstime parameter.
+       * nptl/pthread_rwlock_timedwrlock.c (pthread_rwlock_timedwrlock):
+       Likewise.
+
        nptl: Add POSIX-proposed pthread_cond_clockwait which behaves just
        like pthread_cond_timedwait except it always measures abstime
        against the supplied clockid.

diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
index 89ba21ac7cbebe571469e88df0f9f71b44446d82..120b880acf67c1ac3e0a2d3444801cd4904a49fe 100644 (file)
--- a/nptl/pthread_rwlock_common.c
+++ b/nptl/pthread_rwlock_common.c
@@ -282,6 +282,16 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
 {
   unsigned int r;
 
+  /* Make sure any passed in timeout value is valid.  Note that the previous
+     implementation assumed that this check *must* not be performed if there
+     would in fact be no blocking; however, POSIX only requires that "the
+     validity of the abstime parameter need not be checked if the lock can be
+     immediately acquired" (i.e., we need not but may check it).  */
+  if (abstime
+      && __glibc_unlikely (abstime->tv_nsec >= 1000000000
+      || abstime->tv_nsec < 0))
+    return EINVAL;
+
   /* Make sure we are not holding the rwlock as a writer.  This is a deadlock
      situation we recognize and report.  */
   if (__glibc_unlikely (atomic_load_relaxed (&rwlock->__data.__cur_writer)
@@ -576,6 +586,16 @@ static __always_inline int
 __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock,
     const struct timespec *abstime)
 {
+  /* Make sure any passed in timeout value is valid.  Note that the previous
+     implementation assumed that this check *must* not be performed if there
+     would in fact be no blocking; however, POSIX only requires that "the
+     validity of the abstime parameter need not be checked if the lock can be
+     immediately acquired" (i.e., we need not but may check it).  */
+  if (abstime
+      && __glibc_unlikely (abstime->tv_nsec >= 1000000000
+      || abstime->tv_nsec < 0))
+    return EINVAL;
+
   /* Make sure we are not holding the rwlock as a writer.  This is a deadlock
      situation we recognize and report.  */
   if (__glibc_unlikely (atomic_load_relaxed (&rwlock->__data.__cur_writer)

diff --git a/nptl/pthread_rwlock_timedrdlock.c b/nptl/pthread_rwlock_timedrdlock.c
index aa0053094dddfe5eeee81306f6f0bb15f6341f49..84c1983491aa68321862b1f54cc7e616ffdeb443 100644 (file)
--- a/nptl/pthread_rwlock_timedrdlock.c
+++ b/nptl/pthread_rwlock_timedrdlock.c
@@ -23,15 +23,5 @@ int
 pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock,
     const struct timespec *abstime)
 {
-  /* Make sure the passed in timeout value is valid.  Note that the previous
-     implementation assumed that this check *must* not be performed if there
-     would in fact be no blocking; however, POSIX only requires that "the
-     validity of the abstime parameter need not be checked if the lock can be
-     immediately acquired" (i.e., we need not but may check it).  */
-  /* ??? Just move this to __pthread_rwlock_rdlock_full?  */
-  if (__glibc_unlikely (abstime->tv_nsec >= 1000000000
-      || abstime->tv_nsec < 0))
-    return EINVAL;
-
   return __pthread_rwlock_rdlock_full (rwlock, abstime);
 }

diff --git a/nptl/pthread_rwlock_timedwrlock.c b/nptl/pthread_rwlock_timedwrlock.c
index 3c92e44830647664df43b9f85b272d7c852a22c3..f0b745df02ec1cb7f98f8cb6657cc76784c4001d 100644 (file)
--- a/nptl/pthread_rwlock_timedwrlock.c
+++ b/nptl/pthread_rwlock_timedwrlock.c
@@ -23,15 +23,5 @@ int
 pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock,
     const struct timespec *abstime)
 {
-  /* Make sure the passed in timeout value is valid.  Note that the previous
-     implementation assumed that this check *must* not be performed if there
-     would in fact be no blocking; however, POSIX only requires that "the
-     validity of the abstime parameter need not be checked if the lock can be
-     immediately acquired" (i.e., we need not but may check it).  */
-  /* ??? Just move this to __pthread_rwlock_wrlock_full?  */
-  if (__glibc_unlikely (abstime->tv_nsec >= 1000000000
-      || abstime->tv_nsec < 0))
-    return EINVAL;
-
   return __pthread_rwlock_wrlock_full (rwlock, abstime);
 }