This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 5/6] nptl: Convert some rwlock tests to use libsupport
- From: Mike Crowe <mac at mcrowe dot com>
- To: Florian Weimer <fweimer at redhat dot com>, libc-alpha at sourceware dot org
- Cc: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Date: Sat, 6 Apr 2019 21:15:30 +0100
- Subject: Re: [PATCH 5/6] nptl: Convert some rwlock tests to use libsupport
- References: <cover.13c2d68f411f9010956e8e52edfbe168964e1e5c.1553797867.git-series.mac@mcrowe.com> <cover.13c2d68f411f9010956e8e52edfbe168964e1e5c.1553797867.git-series.mac@mcrowe.com> <286b598d77c33052867a27e071409e8c0e3ce8cf.1553797867.git-series.mac@mcrowe.com> <87o95k3e5o.fsf@oldenburg2.str.redhat.com>
On Friday 05 April 2019 at 13:05:55 +0200, Florian Weimer wrote:
> * Mike Crowe:
>
> > if (pthread_rwlock_wrlock (&r) != 0)
>
> > if (pthread_barrier_init (&b, NULL, 2) != 0)
>
> We have xpthread_rwlock_wrlock and xpthread_barrier_init, which will
> perform the error checking for you. Similarly for a few more functions.
I swapped a few emails with Adhemerval Zanella about that. To me, it felt
like the wrapper functions weren't appropriate when they wrap the very
functions that the test is supposed to be testing. In particular, they
don't provide line number information for the failure. On that basis, the
code you quoted would become:
TEST_COMPARE (pthread_rwlock_wrlock (&r), 0);
xpthread_barrier_init (&b, NULL, 2);
because the file in question is testing rwlock, but can assume that
barriers ought to work.
> > if (clock_gettime (CLOCK_REALTIME, &ts) != 0)
> > - {
> > - puts ("clock_gettime failed");
> > - return 1;
> > - }
> > + FAIL_RET ("clock_gettime failed");
>
> Perhaps it's time to introduce xclock_gettime?
Indeed. I did that after I'd updated this test. I'll fix those.
> > int e = pthread_rwlock_timedrdlock (&r, &ts);
> > if (e == 0)
> > - {
> > - puts ("first rwlock_timedrdlock did not fail");
> > - result = 1;
> > - }
> > + FAIL_PRINT ("first rwlock_timedrdlock did not fail");
> > else if (e != EINVAL)
> > - {
> > - puts ("first rwlock_timedrdlock did not return EINVAL");
> > - result = 1;
> > - }
> > + FAIL_PRINT ("first rwlock_timedrdlock did not return EINVAL");
>
> You could use
>
> TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EINVAL);
True. I thought that the extra messages were useful, but it looks like
TEST_COMPARE provides enough context.
However, I'm less sure about this code from tst-rwlock7.c(do_test):
size_t cnt;
for (cnt = 0; cnt < sizeof (kind) / sizeof (kind[0]); ++cnt)
{
pthread_rwlock_t r;
pthread_rwlockattr_t a;
if (pthread_rwlockattr_init (&a) != 0)
FAIL_EXIT1 ("round %Zu: rwlockattr_t failed\n", cnt);
if (pthread_rwlockattr_setkind_np (&a, kind[cnt]) != 0)
FAIL_EXIT1 ("round %Zu: rwlockattr_setkind failed\n", cnt);
...
If the round numbers are important then I need to keep the messages. If
not, then I can also replace these with TEST_COMPARE. Do you have an
opinion on that?
Thanks.
Mike.