Message ID | 20190317123930.34689-1-mac@mcrowe.com |
---|---|
State | New |
Headers | show |
Series | [v2] nptl/tst-rwlock14: Test pthread_rwlock_timedwrlock correctly | expand |
On 17/03/2019 09:39, Mike Crowe wrote: > The tests for out-of-bounds timespecs first test pthread_rwlock_timedrdlock > and then pthread_rwlock_timedwrlock. It appears that the second and third > tests suffered from copy-and-paste errors and each test > pthread_rwlock_timedrdlock twice in a row. Let's correct that so that > pthread_rwlock_timedwrlock is tested too. > > * nptl/tst-rwlock14.c (do_test): Replace duplicate calls to > pthread_rwlock_timedrdlock with calls to > pthread_rwlock_timedwrlock to ensure that the latter is tested > too. Use new function name in diagnostic messages too. LGTM, but since you are there could you adapt it to libsupport? > --- > ChangeLog | 7 +++++++ > nptl/tst-rwlock14.c | 12 ++++++------ > 2 files changed, 13 insertions(+), 6 deletions(-) > > This patch was originally sent as part of a larger series: > https://sourceware.org/ml/libc-alpha/2019-02/msg00643.html > > diff --git a/ChangeLog b/ChangeLog > index f7c9ee51ad..248bc05351 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,10 @@ > +2019-03-17 Mike Crowe <mac@mcrowe.com> > + > + * nptl/tst-rwlock14.c (do_test): Replace duplicate calls to > + pthread_rwlock_timedrdlock with calls to pthread_rwlock_timedwrlock to > + ensure that the latter is tested too. Use new function name in diagnostic > + messages too. > + > 2019-03-16 Samuel Thibault <samuel.thibault@ens-lyon.org> > > * hurd/hurd/signal.h (_hurd_critical_section_lock): Document how EINTR > diff --git a/nptl/tst-rwlock14.c b/nptl/tst-rwlock14.c > index f8c218395e..6f57169531 100644 > --- a/nptl/tst-rwlock14.c > +++ b/nptl/tst-rwlock14.c > @@ -117,15 +117,15 @@ do_test (void) > result = 1; > } > > - e = pthread_rwlock_timedrdlock (&r, &ts); > + e = pthread_rwlock_timedwrlock (&r, &ts); > if (e == 0) > { > - puts ("second rwlock_timedrdlock did not fail"); > + puts ("second rwlock_timedwrlock did not fail"); > result = 1; > } > else if (e != EINVAL) > { > - puts ("second rwlock_timedrdlock did not return EINVAL"); > + puts ("second rwlock_timedwrlock did not return EINVAL"); > result = 1; > } > > @@ -145,15 +145,15 @@ do_test (void) > result = 1; > } > > - e = pthread_rwlock_timedrdlock (&r, &ts); > + e = pthread_rwlock_timedwrlock (&r, &ts); > if (e == 0) > { > - puts ("third rwlock_timedrdlock did not fail"); > + puts ("third rwlock_timedwrlock did not fail"); > result = 1; > } > else if (e != EINVAL) > { > - puts ("third rwlock_timedrdlock did not return EINVAL"); > + puts ("third rwlock_timedwrlock did not return EINVAL"); > result = 1; > } > >
On Tuesday 19 March 2019 at 18:49:09 -0300, Adhemerval Zanella wrote: > On 17/03/2019 09:39, Mike Crowe wrote: > > The tests for out-of-bounds timespecs first test pthread_rwlock_timedrdlock > > and then pthread_rwlock_timedwrlock. It appears that the second and third > > tests suffered from copy-and-paste errors and each test > > pthread_rwlock_timedrdlock twice in a row. Let's correct that so that > > pthread_rwlock_timedwrlock is tested too. > > > > * nptl/tst-rwlock14.c (do_test): Replace duplicate calls to > > pthread_rwlock_timedrdlock with calls to > > pthread_rwlock_timedwrlock to ensure that the latter is tested > > too. Use new function name in diagnostic messages too. > > LGTM, but since you are there could you adapt it to libsupport? Yes. I plan to convert all the tests that I'm touching to use libsupport and then rebase my clockwait patches on top of that. However, in this case only I think I'd prefer to land this fix on its own first. Thanks for the review. Mike.
On 20/03/2019 14:27, Mike Crowe wrote: > On Tuesday 19 March 2019 at 18:49:09 -0300, Adhemerval Zanella wrote: >> On 17/03/2019 09:39, Mike Crowe wrote: >>> The tests for out-of-bounds timespecs first test pthread_rwlock_timedrdlock >>> and then pthread_rwlock_timedwrlock. It appears that the second and third >>> tests suffered from copy-and-paste errors and each test >>> pthread_rwlock_timedrdlock twice in a row. Let's correct that so that >>> pthread_rwlock_timedwrlock is tested too. >>> >>> * nptl/tst-rwlock14.c (do_test): Replace duplicate calls to >>> pthread_rwlock_timedrdlock with calls to >>> pthread_rwlock_timedwrlock to ensure that the latter is tested >>> too. Use new function name in diagnostic messages too. >> >> LGTM, but since you are there could you adapt it to libsupport? > > Yes. I plan to convert all the tests that I'm touching to use libsupport > and then rebase my clockwait patches on top of that. > > However, in this case only I think I'd prefer to land this fix on its own > first. > > Thanks for the review. Fair enough.
diff --git a/ChangeLog b/ChangeLog index f7c9ee51ad..248bc05351 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2019-03-17 Mike Crowe <mac@mcrowe.com> + + * nptl/tst-rwlock14.c (do_test): Replace duplicate calls to + pthread_rwlock_timedrdlock with calls to pthread_rwlock_timedwrlock to + ensure that the latter is tested too. Use new function name in diagnostic + messages too. + 2019-03-16 Samuel Thibault <samuel.thibault@ens-lyon.org> * hurd/hurd/signal.h (_hurd_critical_section_lock): Document how EINTR diff --git a/nptl/tst-rwlock14.c b/nptl/tst-rwlock14.c index f8c218395e..6f57169531 100644 --- a/nptl/tst-rwlock14.c +++ b/nptl/tst-rwlock14.c @@ -117,15 +117,15 @@ do_test (void) result = 1; } - e = pthread_rwlock_timedrdlock (&r, &ts); + e = pthread_rwlock_timedwrlock (&r, &ts); if (e == 0) { - puts ("second rwlock_timedrdlock did not fail"); + puts ("second rwlock_timedwrlock did not fail"); result = 1; } else if (e != EINVAL) { - puts ("second rwlock_timedrdlock did not return EINVAL"); + puts ("second rwlock_timedwrlock did not return EINVAL"); result = 1; } @@ -145,15 +145,15 @@ do_test (void) result = 1; } - e = pthread_rwlock_timedrdlock (&r, &ts); + e = pthread_rwlock_timedwrlock (&r, &ts); if (e == 0) { - puts ("third rwlock_timedrdlock did not fail"); + puts ("third rwlock_timedwrlock did not fail"); result = 1; } else if (e != EINVAL) { - puts ("third rwlock_timedrdlock did not return EINVAL"); + puts ("third rwlock_timedwrlock did not return EINVAL"); result = 1; }