public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Adhemerval Zanella <azanella@sourceware.org>
To: glibc-cvs@sourceware.org
Subject: [glibc/azanella/master-posix_clock] nptl: pthread_rwlock: Move timeout validation into _full functions
Date: Tue, 25 Jun 2019 22:10:00 -0000	[thread overview]
Message-ID: <20190625221021.40990.qmail@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=126a1454d2a18003e1ac718b5aac84138dd2ca5c

commit 126a1454d2a18003e1ac718b5aac84138dd2ca5c
Author: Mike Crowe <mac@mcrowe.com>
Date:   Mon Jun 24 12:39:02 2019 +0000

    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.

Diff:
---
 ChangeLog                         |  9 +++++++++
 nptl/pthread_rwlock_common.c      | 20 ++++++++++++++++++++
 nptl/pthread_rwlock_timedrdlock.c | 10 ----------
 nptl/pthread_rwlock_timedwrlock.c | 10 ----------
 4 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6c769ca..2509995 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2019-06-21  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 89ba21a..120b880 100644
--- 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 aa00530..84c1983 100644
--- 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 3c92e44..f0b745d 100644
--- 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);
 }


             reply	other threads:[~2019-06-25 22:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-25 22:10 Adhemerval Zanella [this message]
2019-06-26 18:37 Adhemerval Zanella
2019-07-02 17:26 Adhemerval Zanella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190625221021.40990.qmail@sourceware.org \
    --to=azanella@sourceware.org \
    --cc=glibc-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).