Commit Message
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 (INVALID_BARRIER_WAIT): New macro.
(do_test): Use macro to simplify checking barrier exit
code, and actually join the child thread.
---
nptl/tst-setuid3.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
Comments
On 01/19/2016 09:09 PM, Paul E. Murphy wrote:
> 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 (INVALID_BARRIER_WAIT): New macro.
> (do_test): Use macro to simplify checking barrier exit
> code, and actually join the child thread.
Thanks for fixing this.
> +/* True if pthread_barrier_wait returns without an error. */
> +#define INVALID_BARRIER_WAIT(x) ((x != 0) && \
> + (x != PTHREAD_BARRIER_SERIAL_THREAD))
Please use an inline function or put parentheses around the x.
Technically, this change is okay, but Adhemerval needs to decide if it
can go in during the freeze.
Florian
On 19-01-2016 18:17, Florian Weimer wrote:
> On 01/19/2016 09:09 PM, Paul E. Murphy wrote:
>> 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 (INVALID_BARRIER_WAIT): New macro.
>> (do_test): Use macro to simplify checking barrier exit
>> code, and actually join the child thread.
>
> Thanks for fixing this.
>
>> +/* True if pthread_barrier_wait returns without an error. */
>> +#define INVALID_BARRIER_WAIT(x) ((x != 0) && \
>> + (x != PTHREAD_BARRIER_SERIAL_THREAD))
>
> Please use an inline function or put parentheses around the x.
>
> Technically, this change is okay, but Adhemerval needs to decide if it
> can go in during the freeze.
>
> Florian
>
LGTM, thanks.
@@ -33,14 +33,18 @@ static pthread_barrier_t barrier2;
#define FAIL_ERR(fmt, ...) \
do { printf ("FAIL: " fmt ": %m\n", __VA_ARGS__); _exit (1); } while (0)
+/* True if pthread_barrier_wait returns without an error. */
+#define INVALID_BARRIER_WAIT(x) ((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 (INVALID_BARRIER_WAIT (ret))
FAIL ("pthread_barrier_wait (barrier1) (on thread): %d", ret);
ret = pthread_barrier_wait (&barrier2);
- if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+ if (INVALID_BARRIER_WAIT (ret))
FAIL ("pthread_barrier_wait (barrier2) (on thread): %d", ret);
return NULL;
}
@@ -86,7 +90,7 @@ do_test (void)
/* Ensure that the thread is running properly. */
ret = pthread_barrier_wait (&barrier1);
- if (ret != 0)
+ if (INVALID_BARRIER_WAIT (ret))
FAIL ("pthread_barrier_wait (barrier1): %d", ret);
setuid_failure (2);
@@ -97,10 +101,11 @@ do_test (void)
/* Shutdown. */
ret = pthread_barrier_wait (&barrier2);
- if (ret != PTHREAD_BARRIER_SERIAL_THREAD && ret != 0)
+ if (INVALID_BARRIER_WAIT (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;