Re: [PATCH v2 4/4] nptl: Add pthread_timedjoin_np, pthread_clockjoin_np NULL timeout test

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 18/09/2019 05:30, Mike Crowe wrote:
> Passing NULL as the timeout parameter to pthread_timedjoin_np has resulted
> in it behaving like pthread_join for a long time. Since that is now the
> documented behaviour, we ought to test that both it and the new
> pthread_clockjoin_np support it.
> 
> 	* nptl/tst-join14.c: Test passing NULL as abstime parameter of
> 	pthread_timedjoin_np and pthread_clockjoin_np.
> 	* nptl/Makefile: Add tst-join14.c.

LGTM with small nits below.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  ChangeLog         |  5 +++-
>  nptl/Makefile     |  1 +-
>  nptl/tst-join14.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 79 insertions(+)
>  create mode 100644 nptl/tst-join14.c
> 
> diff --git a/ChangeLog b/ChangeLog
> index 8a2166d..048c4b2 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,4 +1,9 @@
>  2019-09-18  Mike Crowe  <mac@mcrowe.com>
> +	* nptl/tst-join14.c: Test passing NULL as abstime parameter of
> +	pthread_timedjoin_np and pthread_clockjoin_np.
> +	* nptl/Makefile: Add tst-join14.c.
> +
> +2019-09-18  Mike Crowe  <mac@mcrowe.com>
>  	* NEWS: Mention addition of pthread_clockjoin_np along with other
>  	"clock" variants of existing "timed" functions.
>  	* sysdeps/nptl/pthread.h: Add declaration of pthread_clockjoin_np.
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 5521a58..3e05b79 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -275,6 +275,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
>  	tst-raise1 \
>  	tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 tst-join7 \
>  	tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \
> +	tst-join14 \
>  	tst-detach1 \
>  	tst-eintr2 tst-eintr3 tst-eintr4 tst-eintr5 \
>  	tst-tsd1 tst-tsd2 tst-tsd3 tst-tsd4 tst-tsd5 tst-tsd6 \

Ok.

> diff --git a/nptl/tst-join14.c b/nptl/tst-join14.c
> new file mode 100644
> index 0000000..261b919
> --- /dev/null
> +++ b/nptl/tst-join14.c
> @@ -0,0 +1,73 @@

Missing one line comment.

> +/* Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/time.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xthread.h>
> +#include <support/xtime.h>
> +
> +
> +#define CLOCK_USE_TIMEDJOIN (-1)
> +
> +
> +static void *
> +tf (void *arg)
> +{
> +  struct timespec ts = make_timespec(0, 100000);
> +  nanosleep(&ts, NULL);
> +
> +  return (void *) 42l;
> +}
> +

Missing space before parenthesis.

> +
> +/* Check that pthread_timedjoin_np and pthread_clockjoin_np wait "forever" if
> + * passed a timeout parameter of NULL. We can't actually wait forever, but we
> + * can be sure that we did at least wait for some time by checking the exit
> + * status of the thread. */
> +static int
> +do_test_clock (clockid_t clockid)
> +{
> +  pthread_t th = xpthread_create (NULL, tf, NULL);
> +
> +  void *status;
> +  int val = (clockid == CLOCK_USE_TIMEDJOIN)
> +    ? pthread_timedjoin_np (th, &status, NULL)
> +    : pthread_clockjoin_np (th, &status, clockid, NULL);
> +  TEST_COMPARE (val, 0);
> +
> +  if (status != (void *) 42l)
> +    FAIL_EXIT1 ("return value %p, expected %p\n", status, (void *) 42l);
> +
> +  return 0;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  do_test_clock (CLOCK_USE_TIMEDJOIN);
> +  do_test_clock (CLOCK_REALTIME);
> +  do_test_clock (CLOCK_MONOTONIC);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> 

Ok.