Patchwork [2/7] nptl: Add POSIX-proposed sem_clockwait

login
register
mail settings
Submitter Mike Crowe
Date March 10, 2019, 4:17 p.m.
Message ID <20190310161715.ylqctrab7rqaum55@mcrowe.com>
Download mbox | patch
Permalink /patch/31806/
State New
Headers show

Comments

Mike Crowe - March 10, 2019, 4:17 p.m.
On Tuesday 05 March 2019 at 10:31:33 -0300, Adhemerval Zanella wrote:
> On 27/02/2019 15:23, Mike Crowe wrote:
> > diff --git a/nptl/tst-sem5.c b/nptl/tst-sem5.c
> > index 2149ade..7f1031d 100644
> > --- a/nptl/tst-sem5.c
> > +++ b/nptl/tst-sem5.c
> > @@ -23,13 +23,15 @@
> >  #include <unistd.h>
> >  #include <sys/time.h>
> 
> As for tst-sem13.c, I think it would be good to adapt to libsupport.

Something like this (before I make the same mistakes on the others):

Subject: [PATCH] nptl: Convert tst-sem5 to use libsupport

* nptl/tst-sem5.c: Remove unused headers. Add <support/check.h>. (do_test)
  Use libsupport test macros rather than hand-coded conditionals and error
  messages. Ensure that sem_init returns zero rather than not -1. Use
  <support/test-driver.c> rather than test-skeleton.c.
---
 nptl/tst-sem5.c | 53 ++++++++++-------------------------------------------
 1 file changed, 10 insertions(+), 43 deletions(-)

--
2.11.0

Thanks.

Mike.
Adhemerval Zanella Netto - March 19, 2019, 5:07 p.m.
On 10/03/2019 13:17, Mike Crowe wrote:
> On Tuesday 05 March 2019 at 10:31:33 -0300, Adhemerval Zanella wrote:
>> On 27/02/2019 15:23, Mike Crowe wrote:
>>> diff --git a/nptl/tst-sem5.c b/nptl/tst-sem5.c
>>> index 2149ade..7f1031d 100644
>>> --- a/nptl/tst-sem5.c
>>> +++ b/nptl/tst-sem5.c
>>> @@ -23,13 +23,15 @@
>>>  #include <unistd.h>
>>>  #include <sys/time.h>
>>
>> As for tst-sem13.c, I think it would be good to adapt to libsupport.
> 
> Something like this (before I make the same mistakes on the others):
> 
> Subject: [PATCH] nptl: Convert tst-sem5 to use libsupport
> 
> * nptl/tst-sem5.c: Remove unused headers. Add <support/check.h>. (do_test)
>   Use libsupport test macros rather than hand-coded conditionals and error
>   messages. Ensure that sem_init returns zero rather than not -1. Use
>   <support/test-driver.c> rather than test-skeleton.c.

I think you can just simplify to:

   * nptl/tst-sem5.c: Use libsupport.

LGTM, thanks.


> ---
>  nptl/tst-sem5.c | 53 ++++++++++-------------------------------------------
>  1 file changed, 10 insertions(+), 43 deletions(-)
> 
> diff --git a/nptl/tst-sem5.c b/nptl/tst-sem5.c
> index 2149adeb12..846813243b 100644
> --- a/nptl/tst-sem5.c
> +++ b/nptl/tst-sem5.c
> @@ -18,11 +18,10 @@
> 
>  #include <errno.h>
>  #include <semaphore.h>
> -#include <stdio.h>
>  #include <time.h>
>  #include <unistd.h>
>  #include <sys/time.h>
> -
> +#include <support/check.h>
> 
>  static int
>  do_test (void)
> @@ -31,23 +30,9 @@ do_test (void)
>    struct timespec ts;
>    struct timeval tv;
> 
> -  if (sem_init (&s, 0, 1) == -1)
> -    {
> -      puts ("sem_init failed");
> -      return 1;
> -    }
> -
> -  if (TEMP_FAILURE_RETRY (sem_wait (&s)) == -1)
> -    {
> -      puts ("sem_wait failed");
> -      return 1;
> -    }
> -
> -  if (gettimeofday (&tv, NULL) != 0)
> -    {
> -      puts ("gettimeofday failed");
> -      return 1;
> -    }
> +  TEST_COMPARE (sem_init (&s, 0, 1), 0);
> +  TEST_COMPARE (TEMP_FAILURE_RETRY (sem_wait (&s)), 0);
> +  TEST_COMPARE (gettimeofday (&tv, NULL), 0);
> 
>    TIMEVAL_TO_TIMESPEC (&tv, &ts);
> 
> @@ -60,34 +45,16 @@ do_test (void)
>      }
> 
>    errno = 0;
> -  if (TEMP_FAILURE_RETRY (sem_timedwait (&s, &ts)) != -1)
> -    {
> -      puts ("sem_timedwait succeeded");
> -      return 1;
> -    }
> -  if (errno != ETIMEDOUT)
> -    {
> -      printf ("sem_timedwait return errno = %d instead of ETIMEDOUT\n",
> -	      errno);
> -      return 1;
> -    }
> +  TEST_COMPARE (TEMP_FAILURE_RETRY (sem_timedwait (&s, &ts)), -1);
> +  TEST_COMPARE (errno, ETIMEDOUT);
> 
>    struct timespec ts2;
> -  if (clock_gettime (CLOCK_REALTIME, &ts2) != 0)
> -    {
> -      puts ("clock_gettime failed");
> -      return 1;
> -    }
> +  TEST_COMPARE (clock_gettime (CLOCK_REALTIME, &ts2), 0);
> 
> -  if (ts2.tv_sec < ts.tv_sec
> -      || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec < ts.tv_nsec))
> -    {
> -      puts ("timeout too short");
> -      return 1;
> -    }
> +  TEST_VERIFY (ts2.tv_sec > ts.tv_sec
> +               || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec > ts.tv_nsec));
> 
>    return 0;
>  }
> 
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> --
> 2.11.0
> 
> Thanks.
> 
> Mike.
>

Patch

diff --git a/nptl/tst-sem5.c b/nptl/tst-sem5.c
index 2149adeb12..846813243b 100644
--- a/nptl/tst-sem5.c
+++ b/nptl/tst-sem5.c
@@ -18,11 +18,10 @@ 

 #include <errno.h>
 #include <semaphore.h>
-#include <stdio.h>
 #include <time.h>
 #include <unistd.h>
 #include <sys/time.h>
-
+#include <support/check.h>

 static int
 do_test (void)
@@ -31,23 +30,9 @@  do_test (void)
   struct timespec ts;
   struct timeval tv;

-  if (sem_init (&s, 0, 1) == -1)
-    {
-      puts ("sem_init failed");
-      return 1;
-    }
-
-  if (TEMP_FAILURE_RETRY (sem_wait (&s)) == -1)
-    {
-      puts ("sem_wait failed");
-      return 1;
-    }
-
-  if (gettimeofday (&tv, NULL) != 0)
-    {
-      puts ("gettimeofday failed");
-      return 1;
-    }
+  TEST_COMPARE (sem_init (&s, 0, 1), 0);
+  TEST_COMPARE (TEMP_FAILURE_RETRY (sem_wait (&s)), 0);
+  TEST_COMPARE (gettimeofday (&tv, NULL), 0);

   TIMEVAL_TO_TIMESPEC (&tv, &ts);

@@ -60,34 +45,16 @@  do_test (void)
     }

   errno = 0;
-  if (TEMP_FAILURE_RETRY (sem_timedwait (&s, &ts)) != -1)
-    {
-      puts ("sem_timedwait succeeded");
-      return 1;
-    }
-  if (errno != ETIMEDOUT)
-    {
-      printf ("sem_timedwait return errno = %d instead of ETIMEDOUT\n",
-	      errno);
-      return 1;
-    }
+  TEST_COMPARE (TEMP_FAILURE_RETRY (sem_timedwait (&s, &ts)), -1);
+  TEST_COMPARE (errno, ETIMEDOUT);

   struct timespec ts2;
-  if (clock_gettime (CLOCK_REALTIME, &ts2) != 0)
-    {
-      puts ("clock_gettime failed");
-      return 1;
-    }
+  TEST_COMPARE (clock_gettime (CLOCK_REALTIME, &ts2), 0);

-  if (ts2.tv_sec < ts.tv_sec
-      || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec < ts.tv_nsec))
-    {
-      puts ("timeout too short");
-      return 1;
-    }
+  TEST_VERIFY (ts2.tv_sec > ts.tv_sec
+               || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec > ts.tv_nsec));

   return 0;
 }

-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>