diff mbox

powerpc: Convert tests to the new support test-driver

Message ID 20161219174939.12893-1-wainersm@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Wainer dos Santos Moschetta Dec. 19, 2016, 5:49 p.m. UTC 
Change the powerpc tests to use <support/test-driver.c>.
Also replace some of pthread calls to its xpthread equivalent.

Tested on ppc64le.

2016-12-15  Wainer dos Santos Moschetta  <wainersm@linux.vnet.ibm.com>

	* sysdeps/powerpc/test-get_hwcap.c: Use <support/test-driver.c>
	instead of test-skeleton.c.
	(do_test): Replaced pthread_create and pthread_join with
	xpthread_create and xpthread_join.
	* sysdeps/powerpc/test-gettimebase.c: Use <support/test-driver.c>
	instead of test-skeleton.c.
	* sysdeps/powerpc/tst-tlsopt-powerpc.c: Likewise.
---
 sysdeps/powerpc/test-get_hwcap.c     | 17 +++++------------
 sysdeps/powerpc/test-gettimebase.c   |  3 +--
 sysdeps/powerpc/tst-tlsopt-powerpc.c |  3 +--
 3 files changed, 7 insertions(+), 16 deletions(-)

Comments

Carlos Eduardo Seo Dec. 19, 2016, 6:43 p.m. UTC | #1 
On 12/19/16, 3:49 PM, "Wainer dos Santos Moschetta" <libc-alpha-owner@sourceware.org on behalf of wainersm@linux.vnet.ibm.com> wrote:

    diff --git a/sysdeps/powerpc/test-get_hwcap.c b/sysdeps/powerpc/test-get_hwcap.c
    index 14fe73b..14093f0 100644
    --- a/sysdeps/powerpc/test-get_hwcap.c
    +++ b/sysdeps/powerpc/test-get_hwcap.c
    @@ -23,6 +23,8 @@
     #include <stdint.h>
     #include <pthread.h>
    
    +#include <support/xthread.h>
    +
     #include <sys/auxv.h>
    
     #include <dl-procinfo.h>
    @@ -160,18 +162,10 @@ do_test (void)
    
       /* Check for other thread.  */
       i++;
    -  if (pthread_create (&threads[i], &attr, t1, (void *)i))
    -    {
    -      printf ("FAIL: error creating thread %ld.\n", i);
    -      return 1;
    -    }
    +  threads[i] = xpthread_create (&attr, t1, (void *)i);
    
       pthread_attr_destroy (&attr);
    -  if (pthread_join (threads[i], &status))
    -    {
    -      printf ("FAIL: error joining thread %ld.\n", i);
    -      return 1;
    -    }
    +  status = xpthread_join (threads[i]);
       if (status)
         {
           return 1;
    @@ -184,5 +178,4 @@ do_test (void)

I don’t think you should omit the FAIL message here. Other than that, LGTM.


--
Carlos Eduardo Seo
Software Engineer - Linux on Power Toolchain
cseo@linux.vnet.ibm.com
Florian Weimer Dec. 19, 2016, 8:01 p.m. UTC | #2 
On 12/19/2016 07:43 PM, Carlos Eduardo Seo wrote:

>        pthread_attr_destroy (&attr);
>     -  if (pthread_join (threads[i], &status))
>     -    {
>     -      printf ("FAIL: error joining thread %ld.\n", i);
>     -      return 1;
>     -    }
>     +  status = xpthread_join (threads[i]);
>        if (status)
>          {
>            return 1;
>     @@ -184,5 +178,4 @@ do_test (void)
>
> I don’t think you should omit the FAIL message here. Other than that, LGTM.

Right.  I could add a TEST_VERIFY macro which would allow you to write:

   TEST_VERIFY (xpthread_join (threads[i]) != NULL);

or

   TEST_VERIFY_FAIL (xpthread_join (threads[i]) != NULL);

(in case execution is not expected to continue).

Thanks,
Florian
Rajalakshmi S Dec. 20, 2016, 5:30 a.m. UTC | #3 
On 12/20/2016 12:13 AM, Carlos Eduardo Seo wrote:
>
> On 12/19/16, 3:49 PM, "Wainer dos Santos Moschetta" <libc-alpha-owner@sourceware.org on behalf of wainersm@linux.vnet.ibm.com> wrote:
>
>      diff --git a/sysdeps/powerpc/test-get_hwcap.c b/sysdeps/powerpc/test-get_hwcap.c
>      index 14fe73b..14093f0 100644
>      --- a/sysdeps/powerpc/test-get_hwcap.c
>      +++ b/sysdeps/powerpc/test-get_hwcap.c
>      @@ -23,6 +23,8 @@
>       #include <stdint.h>
>       #include <pthread.h>
>      
>      +#include <support/xthread.h>
>      +
>       #include <sys/auxv.h>
>      
>       #include <dl-procinfo.h>
>      @@ -160,18 +162,10 @@ do_test (void)
>      
>         /* Check for other thread.  */
>         i++;
>      -  if (pthread_create (&threads[i], &attr, t1, (void *)i))
>      -    {
>      -      printf ("FAIL: error creating thread %ld.\n", i);
>      -      return 1;
>      -    }
>      +  threads[i] = xpthread_create (&attr, t1, (void *)i);
>      
>         pthread_attr_destroy (&attr);
>      -  if (pthread_join (threads[i], &status))
>      -    {
>      -      printf ("FAIL: error joining thread %ld.\n", i);
>      -      return 1;
>      -    }
>      +  status = xpthread_join (threads[i]);
>         if (status)
>           {
>             return 1;
>      @@ -184,5 +178,4 @@ do_test (void)
>
> I don’t think you should omit the FAIL message here. Other than that, LGTM.
>
I think xpthread_check_return() called from xpthread_join()
is already handling it.
> --
> Carlos Eduardo Seo
> Software Engineer - Linux on Power Toolchain
> cseo@linux.vnet.ibm.com
>   
>   
>      
>
>
>
Florian Weimer Dec. 20, 2016, 7:38 a.m. UTC | #4 
On 12/20/2016 06:30 AM, Rajalakshmi Srinivasaraghavan wrote:

>>      +  status = xpthread_join (threads[i]);
>>         if (status)
>>           {
>>             return 1;
>>      @@ -184,5 +178,4 @@ do_test (void)
>>
>> I don’t think you should omit the FAIL message here. Other than that,
>> LGTM.
>>
> I think xpthread_check_return() called from xpthread_join()
> is already handling it.

Not for the status == NULL case.

Florian
Wainer dos Santos Moschetta Dec. 20, 2016, 12:27 p.m. UTC | #5 
On 12/19/2016 06:01 PM, Florian Weimer wrote:
> On 12/19/2016 07:43 PM, Carlos Eduardo Seo wrote:
>
>>        pthread_attr_destroy (&attr);
>>     -  if (pthread_join (threads[i], &status))
>>     -    {
>>     -      printf ("FAIL: error joining thread %ld.\n", i);
>>     -      return 1;
>>     -    }
>>     +  status = xpthread_join (threads[i]);
>>        if (status)
>>          {
>>            return 1;
>>     @@ -184,5 +178,4 @@ do_test (void)
>>
>> I don’t think you should omit the FAIL message here. Other than that, LGTM.
>
> Right.  I could add a TEST_VERIFY macro which would allow you to write:
>
>   TEST_VERIFY (xpthread_join (threads[i]) != NULL);
>
> or
>
>   TEST_VERIFY_FAIL (xpthread_join (threads[i]) != NULL);
>
> (in case execution is not expected to continue).

It sounds good. I'll wait your patch instead of printing a fail message to stdout.

>
>
> Thanks,
> Florian
>
diff mbox

Patch

diff --git a/sysdeps/powerpc/test-get_hwcap.c b/sysdeps/powerpc/test-get_hwcap.c
index 14fe73b..14093f0 100644
--- a/sysdeps/powerpc/test-get_hwcap.c
+++ b/sysdeps/powerpc/test-get_hwcap.c
@@ -23,6 +23,8 @@ 
 #include <stdint.h>
 #include <pthread.h>
 
+#include <support/xthread.h>
+
 #include <sys/auxv.h>
 
 #include <dl-procinfo.h>
@@ -160,18 +162,10 @@  do_test (void)
 
   /* Check for other thread.  */
   i++;
-  if (pthread_create (&threads[i], &attr, t1, (void *)i))
-    {
-      printf ("FAIL: error creating thread %ld.\n", i);
-      return 1;
-    }
+  threads[i] = xpthread_create (&attr, t1, (void *)i);
 
   pthread_attr_destroy (&attr);
-  if (pthread_join (threads[i], &status))
-    {
-      printf ("FAIL: error joining thread %ld.\n", i);
-      return 1;
-    }
+  status = xpthread_join (threads[i]);
   if (status)
     {
       return 1;
@@ -184,5 +178,4 @@  do_test (void)
 
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/sysdeps/powerpc/test-gettimebase.c b/sysdeps/powerpc/test-gettimebase.c
index d6e1324..af502a4 100644
--- a/sysdeps/powerpc/test-gettimebase.c
+++ b/sysdeps/powerpc/test-gettimebase.c
@@ -43,5 +43,4 @@  do_test (void)
   return 1;
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/sysdeps/powerpc/tst-tlsopt-powerpc.c b/sysdeps/powerpc/tst-tlsopt-powerpc.c
index c9a14cb..8ae928a 100644
--- a/sysdeps/powerpc/tst-tlsopt-powerpc.c
+++ b/sysdeps/powerpc/tst-tlsopt-powerpc.c
@@ -8,7 +8,6 @@ 
 COMMON_INT_DEF(foo);
 
 
-#define TEST_FUNCTION do_test ()
 static int
 do_test (void)
 {
@@ -49,4 +48,4 @@  do_test (void)
   return result;
 }
 
-#include "../../test-skeleton.c"
+#include <support/test-driver.c>