Fix nptl/tst-setuid3.c

Message ID 569EC74C.7000301@linux.vnet.ibm.com
State Committed
Headers

Commit Message

Paul E. Murphy Jan. 19, 2016, 11:31 p.m. UTC
  On 01/19/2016 02:33 PM, Florian Weimer wrote:
> On 01/19/2016 09:32 PM, Paul E. Murphy wrote:
>> +/* True if pthread_barrier_wait returns without an error.  */
>> +static inline bool
>> +is_invalid_barrier_ret (int x)
>> +{
>> +  return x != 0 && x != PTHREAD_BARRIER_SERIAL_THREAD;
>> +}
> 
> Sorry, just realized that the comment is wrong, should be “returned
> *with* an error”. :-/

Thanks for point that out.

Sigh, third time is a charm? I rewrote the comment.

> Otherwise, still okay from my point of view.
> 
> Florian
>
  

Comments

Tulio Magno Quites Machado Filho Jan. 22, 2016, 6:31 p.m. UTC | #1
"Paul E. Murphy" <murphyp@linux.vnet.ibm.com> writes:

> On 01/19/2016 02:33 PM, Florian Weimer wrote:
>> On 01/19/2016 09:32 PM, Paul E. Murphy wrote:
>>> +/* True if pthread_barrier_wait returns without an error.  */
>>> +static inline bool
>>> +is_invalid_barrier_ret (int x)
>>> +{
>>> +  return x != 0 && x != PTHREAD_BARRIER_SERIAL_THREAD;
>>> +}
>> 
>> Sorry, just realized that the comment is wrong, should be “returned
>> *with* an error”. :-/
>
> Thanks for point that out.
>
> Sigh, third time is a charm? I rewrote the comment.

Pushed as fad7e4d7 for 2.23.
  

Patch

From e1d9083bc36a707ffcea6df9fd2b432c60f10122 Mon Sep 17 00:00:00 2001
From: "Paul E. Murphy" <murphyp@linux.vnet.ibm.com>
Date: Tue, 19 Jan 2016 13:42:44 -0600
Subject: [PATCH] Fix nptl/tst-setuid3.c

pthread_barrier_wait can return either PTHREAD_BARRIER_SERIAL_THREAD
or 0.  Posix makes no guarantees about which thread return the unique
value.

Additionally, pthread_join was not called despite seemingly checking
for the error.

2016-01-19  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

	* nptl/tst-setuid3.c (is_invalid_barrier_ret): New function.
	(thread_func): Use new function to simplify barrier check.
	(do_test): Use new function to simplify checking barrier exit
	code, and actually join the child thread.
---
 nptl/tst-setuid3.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/nptl/tst-setuid3.c b/nptl/tst-setuid3.c
index e017934..40dbb2a 100644
--- a/nptl/tst-setuid3.c
+++ b/nptl/tst-setuid3.c
@@ -33,14 +33,21 @@  static pthread_barrier_t barrier2;
 #define FAIL_ERR(fmt, ...) \
   do { printf ("FAIL: " fmt ": %m\n", __VA_ARGS__); _exit (1); } while (0)
 
+/* True if x is not a successful return code from pthread_barrier_wait.  */
+static inline bool
+is_invalid_barrier_ret (int x)
+{
+  return x != 0 && x != PTHREAD_BARRIER_SERIAL_THREAD;
+}
+
 static void *
 thread_func (void *ctx __attribute__ ((unused)))
 {
   int ret = pthread_barrier_wait (&barrier1);
-  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+  if (is_invalid_barrier_ret (ret))
     FAIL ("pthread_barrier_wait (barrier1) (on thread): %d", ret);
   ret = pthread_barrier_wait (&barrier2);
-  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+  if (is_invalid_barrier_ret (ret))
     FAIL ("pthread_barrier_wait (barrier2) (on thread): %d", ret);
   return NULL;
 }
@@ -86,7 +93,7 @@  do_test (void)
 
   /* Ensure that the thread is running properly.  */
   ret = pthread_barrier_wait (&barrier1);
-  if (ret != 0)
+  if (is_invalid_barrier_ret (ret))
     FAIL ("pthread_barrier_wait (barrier1): %d", ret);
 
   setuid_failure (2);
@@ -97,10 +104,11 @@  do_test (void)
 
   /* Shutdown.  */
   ret = pthread_barrier_wait (&barrier2);
-  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+  if (is_invalid_barrier_ret (ret))
     FAIL ("pthread_barrier_wait (barrier2): %d", ret);
 
-  if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+  ret = pthread_join (thread, NULL);
+  if (ret != 0)
     FAIL ("pthread_join: %d", ret);
 
   return 0;
-- 
2.4.3