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: Florian Weimer <fweimer at redhat dot com>
- To: Mike Crowe <mac at mcrowe dot com>
- Cc: libc-alpha at sourceware dot org, Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Date: Mon, 08 Apr 2019 09:47:32 +0200
- 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> <20190406201530.wdhuxcbifseabyfy@mcrowe.com>
* Mike Crowe:
> 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.
I don't have a strong opinion here. I think we should have proper error
testing in tests, and also report a little bit more detail on unexpected
errors. The TEST_COMPARE approach actually provides more information
than the x* functions (because the latter do not show the location of
the call), so that's fine as well.
> True. I thought that the extra messages were useful, but it looks like
> TEST_COMPARE provides enough context.
Right.
> 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?
I don't think I've ever seen tst-rwlock7 fail, so I don't know if the
round numbers are useful for diagnosing test failures.
Thanks,
Florian