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

  * support/check.h: Introduce FAIL_THREAD_EXIT1 and FAIL_PRINT
	macros.

	* nptl/tst-rwlock6.c, nptl/tst-rwlock7.c, nptl/tst-rwlock9.c,
	nptl/tst-rwlock14.c: Use libsupport.
---
 ChangeLog           |   8 +++-
 nptl/tst-rwlock14.c |  91 +++++++-------------------------------
 nptl/tst-rwlock6.c  | 108 ++++++++-------------------------------------
 nptl/tst-rwlock7.c  |  82 +++++++---------------------------
 nptl/tst-rwlock9.c  |  88 +++++++------------------------------
 support/check.h     |  14 ++++++-
 6 files changed, 99 insertions(+), 292 deletions(-)
  

On 29/03/2019 01:31, Mike Crowe wrote:
> * support/check.h: Introduce FAIL_THREAD_EXIT1 and FAIL_PRINT
> 	macros.
> 
> 	* nptl/tst-rwlock6.c, nptl/tst-rwlock7.c, nptl/tst-rwlock9.c,
> 	nptl/tst-rwlock14.c: Use libsupport.
> ---
>  ChangeLog           |   8 +++-
>  nptl/tst-rwlock14.c |  91 +++++++-------------------------------
>  nptl/tst-rwlock6.c  | 108 ++++++++-------------------------------------
>  nptl/tst-rwlock7.c  |  82 +++++++---------------------------
>  nptl/tst-rwlock9.c  |  88 +++++++------------------------------
>  support/check.h     |  14 ++++++-
>  6 files changed, 99 insertions(+), 292 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 5507086..5a03d42 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,13 @@
>  2019-03-28  Mike Crowe  <mac@mcrowe.com>
>  
> +	* support/check.h: Introduce FAIL_THREAD_EXIT1 and FAIL_PRINT
> +	macros.
> +
> +	* nptl/tst-rwlock6.c, nptl/tst-rwlock7.c, nptl/tst-rwlock9.c,
> +	nptl/tst-rwlock14.c: Use libsupport.
> +
> +2019-03-28  Mike Crowe  <mac@mcrowe.com>
> +
>  	* nptl/tst-rwlock6.c: Fix small bug in timeout-checking code that
>  	could erroneously pass if the function incorrectly took more than a
>  	second.
> diff --git a/nptl/tst-rwlock14.c b/nptl/tst-rwlock14.c
> index 6f57169..fb0c0e3 100644
> --- a/nptl/tst-rwlock14.c
> +++ b/nptl/tst-rwlock14.c
> @@ -21,6 +21,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <time.h>
> +#include <support/check.h>
>  
>  
>  static pthread_barrier_t b;
> @@ -32,10 +33,7 @@ tf (void *arg)
>  {
>    /* Lock the read-write lock.  */
>    if (pthread_rwlock_wrlock (&r) != 0)
> -    {
> -      puts ("tf: cannot lock rwlock");
> -      exit (EXIT_FAILURE);
> -    }
> +    FAIL_EXIT1 ("tf: cannot lock rwlock");
>  
>    pthread_t mt = *(pthread_t *) arg;
>  
> @@ -51,28 +49,18 @@ tf (void *arg)
>  static int
>  do_test (void)
>  {
> -  int result = 0;
>    struct timespec ts;
>  
>    if (clock_gettime (CLOCK_REALTIME, &ts) != 0)
> -    {
> -      puts ("clock_gettime failed");
> -      return 1;
> -    }
> +    FAIL_RET ("clock_gettime failed");
>  
>    if (pthread_barrier_init (&b, NULL, 2) != 0)
> -    {
> -      puts ("barrier_init failed");
> -      return 1;
> -    }
> +    FAIL_RET ("barrier_init failed");
>  
>    pthread_t me = pthread_self ();
>    pthread_t th;
>    if (pthread_create (&th, NULL, tf, &me) != 0)
> -    {
> -      puts ("create failed");
> -      return 1;
> -    }
> +    FAIL_RET ("create failed");
>  
>    /* Wait until the rwlock is locked.  */
>    pthread_barrier_wait (&b);
> @@ -81,53 +69,29 @@ do_test (void)
>  
>    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");
>  
>    e = pthread_rwlock_timedwrlock (&r, &ts);
>    if (e == 0)
> -    {
> -      puts ("first rwlock_timedwrlock did not fail");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("first rwlock_timedwrlock did not fail");
>    else if (e != EINVAL)
> -    {
> -      puts ("first rwlock_timedwrlock did not return EINVAL");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("first rwlock_timedwrlock did not return EINVAL");
>  
>    ts.tv_nsec = 1000000000;
>  
>    e = pthread_rwlock_timedrdlock (&r, &ts);
>    if (e == 0)
> -    {
> -      puts ("second rwlock_timedrdlock did not fail");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("second rwlock_timedrdlock did not fail");
>    else if (e != EINVAL)
> -    {
> -      puts ("second rwlock_timedrdlock did not return EINVAL");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("second rwlock_timedrdlock did not return EINVAL");
>  
>    e = pthread_rwlock_timedwrlock (&r, &ts);
>    if (e == 0)
> -    {
> -      puts ("second rwlock_timedwrlock did not fail");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("second rwlock_timedwrlock did not fail");
>    else if (e != EINVAL)
> -    {
> -      puts ("second rwlock_timedwrlock did not return EINVAL");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("second rwlock_timedwrlock did not return EINVAL");
>  
>    ts.tv_nsec = (__typeof (ts.tv_nsec)) 0x100001000LL;
>    if ((__typeof (ts.tv_nsec)) 0x100001000LL != 0x100001000LL)
> @@ -135,34 +99,17 @@ do_test (void)
>  
>    e = pthread_rwlock_timedrdlock (&r, &ts);
>    if (e == 0)
> -    {
> -      puts ("third rwlock_timedrdlock did not fail");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("third rwlock_timedrdlock did not fail");
>    else if (e != EINVAL)
> -    {
> -      puts ("third rwlock_timedrdlock did not return EINVAL");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("third rwlock_timedrdlock did not return EINVAL");
>  
>    e = pthread_rwlock_timedwrlock (&r, &ts);
>    if (e == 0)
> -    {
> -      puts ("third rwlock_timedwrlock did not fail");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("third rwlock_timedwrlock did not fail");
>    else if (e != EINVAL)
> -    {
> -      puts ("third rwlock_timedwrlock did not return EINVAL");
> -      result = 1;
> -    }
> +    FAIL_PRINT ("third rwlock_timedwrlock did not return EINVAL");
>  
> -  if (result == 0)
> -    puts ("no bugs");
> -
> -  return result;
> +  return 0;
>  }
>  
> -
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/nptl/tst-rwlock6.c b/nptl/tst-rwlock6.c
> index e57f045..944d51f 100644
> --- a/nptl/tst-rwlock6.c
> +++ b/nptl/tst-rwlock6.c
> @@ -22,6 +22,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <sys/time.h>
> +#include <support/check.h>
>  #include <support/timespec.h>
>  
>  
> @@ -32,7 +33,6 @@ static int kind[] =
>      PTHREAD_RWLOCK_PREFER_WRITER_NP,
>    };
>  
> -
>  static void *
>  tf (void *arg)
>  {
> @@ -49,31 +49,13 @@ tf (void *arg)
>  
>    int err = pthread_rwlock_timedrdlock (r, &ts_timeout);
>    if (err == 0)
> -    {
> -      puts ("rwlock_timedrdlock returned");
> -      pthread_exit ((void *) 1l);
> -    }
> +    FAIL_THREAD_EXIT1 ("rwlock_timedrdlock returned");
>  
> -  if (err != ETIMEDOUT)
> -    {
> -      printf ("err = %s (%d), expected %s (%d)\n",
> -	      strerror (err), err, strerror (ETIMEDOUT), ETIMEDOUT);
> -      pthread_exit ((void *) 1l);
> -    }
> +  TEST_COMPARE (err, ETIMEDOUT);
>  
>    puts ("1st child timedrdlock done");
>  
> -  struct timespec ts_end;
> -  (void) clock_gettime (CLOCK_REALTIME, &ts_end);
> -
> -  struct timespec ts_duration;
> -  timespec_sub (&ts_duration, &ts_end, &ts_start);
> -
> -  if (ts_duration.tv_sec !=0 || ts_duration.tv_nsec < 200000000)
> -    {
> -      puts ("timeout too short");
> -      pthread_exit ((void *) 1l);
> -    }
> +  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts_timeout);
>  
>    (void) clock_gettime (CLOCK_REALTIME, &ts_timeout);
>    ts_timeout.tv_sec += 10;
> @@ -82,15 +64,9 @@ tf (void *arg)
>  
>    err = pthread_rwlock_timedrdlock (r, &ts_timeout);
>    if (err == 0)
> -    {
> -      puts ("2nd timedrdlock succeeded");
> -      pthread_exit ((void *) 1l);
> -    }
> -  if (err != EINVAL)
> -    {
> -      puts ("2nd timedrdlock did not return EINVAL");
> -      pthread_exit ((void *) 1l);
> -    }
> +    FAIL_THREAD_EXIT1 ("2nd timedrdlock succeeded");
> +
> +  TEST_COMPARE (err, EINVAL);
>  
>    puts ("2nd child timedrdlock done");
>  
> @@ -108,28 +84,16 @@ do_test (void)
>        pthread_rwlockattr_t a;
>  
>        if (pthread_rwlockattr_init (&a) != 0)
> -	{
> -	  printf ("round %Zu: rwlockattr_t failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_t failed\n", cnt);
>  
>        if (pthread_rwlockattr_setkind_np (&a, kind[cnt]) != 0)
> -	{
> -	  printf ("round %Zu: rwlockattr_setkind failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_setkind failed\n", cnt);
>  
>        if (pthread_rwlock_init (&r, &a) != 0)
> -	{
> -	  printf ("round %Zu: rwlock_init failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlock_init failed\n", cnt);
>  
>        if (pthread_rwlockattr_destroy (&a) != 0)
> -	{
> -	  printf ("round %Zu: rwlockattr_destroy failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_destroy failed\n", cnt);
>  
>        struct timespec ts;
>        (void) clock_gettime (CLOCK_REALTIME, &ts);
> @@ -138,10 +102,7 @@ do_test (void)
>        /* Get a write lock.  */
>        int e = pthread_rwlock_timedwrlock (&r, &ts);
>        if (e != 0)
> -	{
> -	  printf ("round %Zu: rwlock_timedwrlock failed (%d)\n", cnt, e);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlock_timedwrlock failed (%d)\n", cnt, e);
>  
>        puts ("1st timedwrlock done");
>  
> @@ -149,15 +110,8 @@ do_test (void)
>        ++ts.tv_sec;
>        e = pthread_rwlock_timedrdlock (&r, &ts);
>        if (e == 0)
> -	{
> -	  puts ("timedrdlock succeeded");
> -	  exit (1);
> -	}
> -      if (e != EDEADLK)
> -	{
> -	  puts ("timedrdlock did not return EDEADLK");
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("timedrdlock succeeded");
> +      TEST_COMPARE (e, EDEADLK);
>  
>        puts ("1st timedrdlock done");
>  
> @@ -165,50 +119,30 @@ do_test (void)
>        ++ts.tv_sec;
>        e = pthread_rwlock_timedwrlock (&r, &ts);
>        if (e == 0)
> -	{
> -	  puts ("2nd timedwrlock succeeded");
> -	  exit (1);
> -	}
> -      if (e != EDEADLK)
> -	{
> -	  puts ("2nd timedwrlock did not return EDEADLK");
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("2nd timedwrlock succeeded");
> +      TEST_COMPARE(e, EDEADLK);
>  
>        puts ("2nd timedwrlock done");
>  
>        pthread_t th;
>        if (pthread_create (&th, NULL, tf, &r) != 0)
> -	{
> -	  printf ("round %Zu: create failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: create failed\n", cnt);
>  
>        puts ("started thread");
>  
>        void *status;
>        if (pthread_join (th, &status) != 0)
> -	{
> -	  printf ("round %Zu: join failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: join failed\n", cnt);
>        if (status != NULL)
> -	{
> -	  printf ("failure in round %Zu\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("failure in round %Zu\n", cnt);
>  
>        puts ("joined thread");
>  
>        if (pthread_rwlock_destroy (&r) != 0)
> -	{
> -	  printf ("round %Zu: rwlock_destroy failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlock_destroy failed\n", cnt);
>      }
>  
>    return 0;
>  }
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/nptl/tst-rwlock7.c b/nptl/tst-rwlock7.c
> index 1c64969..a57a41f 100644
> --- a/nptl/tst-rwlock7.c
> +++ b/nptl/tst-rwlock7.c
> @@ -22,6 +22,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <sys/time.h>
> +#include <support/check.h>
>  #include <support/timespec.h>
>  
>  
> @@ -46,29 +47,14 @@ tf (void *arg)
>  
>    int err = pthread_rwlock_timedwrlock (r, &ts_timeout);
>    if (err == 0)
> -    {
> -      puts ("rwlock_timedwrlock returned");
> -      pthread_exit ((void *) 1l);
> -    }
> +    FAIL_THREAD_EXIT1 ("rwlock_timedwrlock returned");
>  
>    if (err != ETIMEDOUT)
> -    {
> -      printf ("err = %s (%d), expected %s (%d)\n",
> +    FAIL_THREAD_EXIT1 ("err = %s (%d), expected %s (%d)\n",
>  	      strerror (err), err, strerror (ETIMEDOUT), ETIMEDOUT);
> -      pthread_exit ((void *) 1l);
> -    }
>    puts ("child: timedwrlock failed with ETIMEDOUT");
>  
> -  struct timespec ts_end;
> -  (void) clock_gettime (CLOCK_REALTIME, &ts_end);
> -  struct timespec ts_diff;
> -  timespec_sub (&ts_diff, &ts_end, &ts_start);
> -
> -  if (ts_diff.tv_sec != 0 || ts_diff.tv_nsec < 200000000)
> -    {
> -      puts ("timeout too short");
> -      pthread_exit ((void *) 1l);
> -    }
> +  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts_timeout);
>  
>    struct timespec ts_invalid;
>    (void) clock_gettime (CLOCK_REALTIME, &ts_invalid);
> @@ -78,15 +64,10 @@ tf (void *arg)
>  
>    err = pthread_rwlock_timedwrlock (r, &ts_invalid);
>    if (err == 0)
> -    {
> -      puts ("2nd timedwrlock succeeded");
> -      pthread_exit ((void *) 1l);
> -    }
> +    FAIL_THREAD_EXIT1 ("2nd timedwrlock succeeded");
>    if (err != EINVAL)
> -    {
> -      puts ("2nd timedwrlock did not return EINVAL");
> -      pthread_exit ((void *) 1l);
> -    }
> +    FAIL_THREAD_EXIT1 ("2nd timedwrlock did not return EINVAL");
> +
>    puts ("child: timedwrlock failed with EINVAL");
>  
>    return NULL;
> @@ -103,28 +84,16 @@ do_test (void)
>        pthread_rwlockattr_t a;
>  
>        if (pthread_rwlockattr_init (&a) != 0)
> -	{
> -	  printf ("round %Zu: rwlockattr_t failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_t failed\n", cnt);
>  
>        if (pthread_rwlockattr_setkind_np (&a, kind[cnt]) != 0)
> -	{
> -	  printf ("round %Zu: rwlockattr_setkind failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_setkind failed\n", cnt);
>  
>        if (pthread_rwlock_init (&r, &a) != 0)
> -	{
> -	  printf ("round %Zu: rwlock_init failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlock_init failed\n", cnt);
>  
>        if (pthread_rwlockattr_destroy (&a) != 0)
> -	{
> -	  printf ("round %Zu: rwlockattr_destroy failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_destroy failed\n", cnt);
>  
>        struct timespec ts;
>        (void) clock_gettime (CLOCK_REALTIME, &ts);
> @@ -133,40 +102,25 @@ do_test (void)
>  
>        /* Get a read lock.  */
>        if (pthread_rwlock_timedrdlock (&r, &ts) != 0)
> -	{
> -	  printf ("round %Zu: rwlock_timedrdlock failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlock_timedrdlock failed\n", cnt);
> +
>        printf ("%zu: got timedrdlock\n", cnt);
>  
>        pthread_t th;
>        if (pthread_create (&th, NULL, tf, &r) != 0)
> -	{
> -	  printf ("round %Zu: create failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: create failed\n", cnt);
>  
>        void *status;
>        if (pthread_join (th, &status) != 0)
> -	{
> -	  printf ("round %Zu: join failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: join failed\n", cnt);
>        if (status != NULL)
> -	{
> -	  printf ("failure in round %Zu\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("failure in round %Zu\n", cnt);
>  
>        if (pthread_rwlock_destroy (&r) != 0)
> -	{
> -	  printf ("round %Zu: rwlock_destroy failed\n", cnt);
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("round %Zu: rwlock_destroy failed\n", cnt);
>      }
>  
>    return 0;
>  }
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/nptl/tst-rwlock9.c b/nptl/tst-rwlock9.c
> index ff15f90..bca46d9 100644
> --- a/nptl/tst-rwlock9.c
> +++ b/nptl/tst-rwlock9.c
> @@ -24,6 +24,8 @@
>  #include <time.h>
>  #include <unistd.h>
>  #include <sys/time.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
>  
>  
>  #define NWRITERS 15
> @@ -31,8 +33,8 @@
>  #define NREADERS 15
>  #define READTRIES 15
>  
> -#define TIMEOUT 1000000
> -#define DELAY   1000000
> +static const struct timespec timeout = {0,1000000};
> +static const struct timespec delay = {0, 1000000};
>  
>  #ifndef KIND
>  # define KIND PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
> @@ -45,12 +47,8 @@ static void *
>  writer_thread (void *nr)
>  {
>    struct timespec ts;
> -  struct timespec delay;
>    int n;
>  
> -  delay.tv_sec = 0;
> -  delay.tv_nsec = DELAY;
> -
>    for (n = 0; n < WRITETRIES; ++n)
>      {
>        int e;
> @@ -58,21 +56,14 @@ writer_thread (void *nr)
>  	{
>  	  (void) clock_gettime (CLOCK_REALTIME, &ts);
>  
> -	  ts.tv_nsec += 2 * TIMEOUT;
> -	  if (ts.tv_nsec >= 1000000000)
> -	    {
> -	      ts.tv_nsec -= 1000000000;
> -	      ++ts.tv_sec;
> -	    }
> +          timespec_add(&ts, &ts, &timeout);
> +          timespec_add(&ts, &ts, &timeout);
>  
>  	  printf ("writer thread %ld tries again\n", (long int) nr);
>  
>  	  e = pthread_rwlock_timedwrlock (&lock, &ts);
>  	  if (e != 0 && e != ETIMEDOUT)
> -	    {
> -	      puts ("timedwrlock failed");
> -	      exit (1);
> -	    }
> +            FAIL_EXIT1 ("timedwrlock failed");
>  	}
>        while (e == ETIMEDOUT);
>  
> @@ -81,10 +72,7 @@ writer_thread (void *nr)
>        nanosleep (&delay, NULL);
>  
>        if (pthread_rwlock_unlock (&lock) != 0)
> -	{
> -	  puts ("unlock for writer failed");
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("unlock for writer failed");
>  
>        printf ("writer thread %ld released\n", (long int) nr);
>      }
> @@ -97,12 +85,8 @@ static void *
>  reader_thread (void *nr)
>  {
>    struct timespec ts;
> -  struct timespec delay;
>    int n;
>  
> -  delay.tv_sec = 0;
> -  delay.tv_nsec = DELAY;
> -
>    for (n = 0; n < READTRIES; ++n)
>      {
>        int e;
> @@ -110,21 +94,13 @@ reader_thread (void *nr)
>  	{
>  	  (void) clock_gettime (CLOCK_REALTIME, &ts);
>  
> -	  ts.tv_nsec += TIMEOUT;
> -	  if (ts.tv_nsec >= 1000000000)
> -	    {
> -	      ts.tv_nsec -= 1000000000;
> -	      ++ts.tv_sec;
> -	    }
> +          timespec_add(&ts, &ts, &timeout);
>  
>  	  printf ("reader thread %ld tries again\n", (long int) nr);
>  
>  	  e = pthread_rwlock_timedrdlock (&lock, &ts);
>  	  if (e != 0 && e != ETIMEDOUT)
> -	    {
> -	      puts ("timedrdlock failed");
> -	      exit (1);
> -	    }
> +            FAIL_EXIT1 ("timedrdlock failed");
>  	}
>        while (e == ETIMEDOUT);
>  
> @@ -133,10 +109,7 @@ reader_thread (void *nr)
>        nanosleep (&delay, NULL);
>  
>        if (pthread_rwlock_unlock (&lock) != 0)
> -	{
> -	  puts ("unlock for reader failed");
> -	  exit (1);
> -	}
> +        FAIL_EXIT1 ("unlock for reader failed");
>  
>        printf ("reader thread %ld released\n", (long int) nr);
>      }
> @@ -155,22 +128,13 @@ do_test (void)
>    pthread_rwlockattr_t a;
>  
>    if (pthread_rwlockattr_init (&a) != 0)
> -    {
> -      puts ("rwlockattr_t failed");
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("rwlockattr_t failed");
>  
>    if (pthread_rwlockattr_setkind_np (&a, KIND) != 0)
> -    {
> -      puts ("rwlockattr_setkind failed");
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("rwlockattr_setkind failed");
>  
>    if (pthread_rwlock_init (&lock, &a) != 0)
> -    {
> -      puts ("rwlock_init failed");
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("rwlock_init failed");
>  
>    /* Make standard error the same as standard output.  */
>    dup2 (1, 2);
> @@ -181,37 +145,23 @@ do_test (void)
>    for (n = 0; n < NWRITERS; ++n)
>      if (pthread_create (&thwr[n], NULL, writer_thread,
>  			(void *) (long int) n) != 0)
> -      {
> -	puts ("writer create failed");
> -	exit (1);
> -      }
> +      FAIL_EXIT1 ("writer create failed");
>  
>    for (n = 0; n < NREADERS; ++n)
>      if (pthread_create (&thrd[n], NULL, reader_thread,
>  			(void *) (long int) n) != 0)
> -      {
> -	puts ("reader create failed");
> -	exit (1);
> -      }
> +      FAIL_EXIT1 ("reader create failed");
>  
>    /* Wait for all the threads.  */
>    for (n = 0; n < NWRITERS; ++n)
>      if (pthread_join (thwr[n], &res) != 0)
> -      {
> -	puts ("writer join failed");
> -	exit (1);
> -      }
> +      FAIL_EXIT1 ("writer join failed");
>    for (n = 0; n < NREADERS; ++n)
>      if (pthread_join (thrd[n], &res) != 0)
> -      {
> -	puts ("reader join failed");
> -	exit (1);
> -      }
> +      FAIL_EXIT1 ("reader join failed");
>  
>    return 0;
>  }
>  
> -#undef TIMEOUT
>  #define TIMEOUT 30
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/support/check.h b/support/check.h
> index eb3d248..61c9436 100644
> --- a/support/check.h
> +++ b/support/check.h
> @@ -23,6 +23,12 @@
>  
>  __BEGIN_DECLS
>  
> +/* Record a test failure, print the failure message to standard
> +   output, and continue executing. */
> +
> +#define FAIL_PRINT(...) \
> +  support_print_failure_impl (__FILE__, __LINE__, __VA_ARGS__)
> +

Not sure if we really need, since we already have TEST_VERIFY.  For instance
on the change

   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");

I think we can just use

TEST_VERIFY (pthread_rwlock_timedrdlock (&r, &ts) != 0)

It would give us the custom made error message, but I also think TEST_VERIFY
already provides enough information to correct pinpoint to issue if it was
the case.


>  /* Record a test failure, print the failure message to standard output
>     and return 1.  */
>  #define FAIL_RET(...) \
> @@ -44,6 +50,14 @@ __BEGIN_DECLS
>  #define FAIL_UNSUPPORTED(...) \
>    support_exit_failure_impl (77, __FILE__, __LINE__, __VA_ARGS__)
>  
> +/* Record a test failure, print the failure message and exit the
> +   current thread with status 1. */
> +#define FAIL_THREAD_EXIT1(...) \
> +  ({ \
> +    support_print_failure_impl (__FILE__, __LINE__, __VA_ARGS__);       \
> +    pthread_exit ((void *) 1L);                                         \
> +  })
> +

As for before I think we might just use FAIL_EXIT1/FAIL_EXIT and terminate
the process in such cases.  I think adding pthread.h as requisite, even
implicit within a macro, is not really required.

>  /* Record a test failure (but continue executing) if EXPR evaluates to
>     false.  */
>  #define TEST_VERIFY(expr)                                       \
>
  

* 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.

>    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?

>    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);

instead.

Thanks,
Florian
  

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.
  

* 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
  

On Monday 08 April 2019 at 09:47:32 +0200, Florian Weimer wrote:
> * Mike Crowe:
> > 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.

The patch series I posted yesterday[1] leaves these messages showing the
round number (as above.) I'm happy to change them to use TEST_COMPARE
instead if required.

Thanks.

Mike.

[1] https://sourceware.org/ml/libc-alpha/2019-04/msg00161.html