Message ID | 7bc84c321f0ef19f565fb411cc54b50eb5c83cdc.1560875584.git-series.mac@mcrowe.com |
---|---|
State | New |
Headers | show |
Series | nptl: Implement POSIX-proposed _clock variants of existing _timed functions | expand |
On 18/06/2019 13:33, Mike Crowe wrote: > 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. LGTM, I will commit this for you. > --- > ChangeLog | 14 ++++++++++++++ > nptl/pthread_rwlock_common.c | 20 ++++++++++++++++++++ > nptl/pthread_rwlock_timedrdlock.c | 10 ---------- > nptl/pthread_rwlock_timedwrlock.c | 10 ---------- > 4 files changed, 34 insertions(+), 20 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 8386d5b..452d047 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,5 +1,19 @@ > 2019-05-27 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. > + > +2019-05-27 Mike Crowe <mac@mcrowe.com> > + > 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); > } >
diff --git a/ChangeLog b/ChangeLog index 8386d5b..452d047 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,19 @@ 2019-05-27 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. + +2019-05-27 Mike Crowe <mac@mcrowe.com> + 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); }