This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 5/6] nptl: Convert some rwlock tests to use libsupport


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]