| Message ID | 569E97E2.1000307@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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.
diff --git a/nptl/tst-setuid3.c b/nptl/tst-setuid3.c index e017934..69667d8 100644 --- a/nptl/tst-setuid3.c +++ b/nptl/tst-setuid3.c @@ -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;