[2/6] nptl: Convert tst-cond11.c to use libsupport
Commit Message
* support/timespec.h: Create header to provide timespec helper
functions from sysdeps/pthread/posix-timer.h and macros in the
style of those in check.h.
* nptl/tst-cond11.c: Use libsupport.
---
ChangeLog | 8 ++-
nptl/tst-cond11.c | 169 +++++++++-------------------------------------
support/timespec.h | 115 +++++++++++++++++++++++++++++++-
3 files changed, 159 insertions(+), 133 deletions(-)
create mode 100644 support/timespec.h
Comments
On Thursday 28 March 2019 at 18:31:08 +0000, Mike Crowe wrote:
> * support/timespec.h: Create header to provide timespec helper
> functions from sysdeps/pthread/posix-timer.h and macros in the
> style of those in check.h.
>
> * nptl/tst-cond11.c: Use libsupport.
> ---
> ChangeLog | 8 ++-
> nptl/tst-cond11.c | 169 +++++++++-------------------------------------
> support/timespec.h | 115 +++++++++++++++++++++++++++++++-
> 3 files changed, 159 insertions(+), 133 deletions(-)
> create mode 100644 support/timespec.h
>
> diff --git a/ChangeLog b/ChangeLog
> index ff4dabd..5ceb3a7 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,11 @@
> +2019-03-20 Mike Crowe <mac@mcrowe.com>
> +
> + * support/timespec.h: Create header to provide timespec helper
> + functions from sysdeps/pthread/posix-timer.h and macros in the
> + style of those in check.h.
> +
> + * nptl/tst-cond11.c: Use libsupport.
> +
> 2019-03-28 Mike Crowe <mac@mcrowe.com>
>
> * support/xclock_gettime.c (xclock_gettime): New file. Provide
> diff --git a/nptl/tst-cond11.c b/nptl/tst-cond11.c
> index 97a8bd0..3bc4ff4 100644
> --- a/nptl/tst-cond11.c
> +++ b/nptl/tst-cond11.c
> @@ -21,6 +21,10 @@
> #include <stdio.h>
> #include <time.h>
> #include <unistd.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xthread.h>
> +#include <support/xtime.h>
>
>
> #if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0
> @@ -34,132 +38,36 @@ run_test (clockid_t cl)
>
> printf ("clock = %d\n", (int) cl);
>
> - if (pthread_condattr_init (&condattr) != 0)
> - {
> - puts ("condattr_init failed");
> - return 1;
> - }
> -
> - if (pthread_condattr_setclock (&condattr, cl) != 0)
> - {
> - puts ("condattr_setclock failed");
> - return 1;
> - }
> + TEST_COMPARE (pthread_condattr_init (&condattr), 0);
> + TEST_COMPARE (pthread_condattr_setclock (&condattr, cl), 0);
>
> clockid_t cl2;
> - if (pthread_condattr_getclock (&condattr, &cl2) != 0)
> - {
> - puts ("condattr_getclock failed");
> - return 1;
> - }
> - if (cl != cl2)
> - {
> - printf ("condattr_getclock returned wrong value: %d, expected %d\n",
> - (int) cl2, (int) cl);
> - return 1;
> - }
> -
> - if (pthread_cond_init (&cond, &condattr) != 0)
> - {
> - puts ("cond_init failed");
> - return 1;
> - }
> -
> - if (pthread_condattr_destroy (&condattr) != 0)
> - {
> - puts ("condattr_destroy failed");
> - return 1;
> - }
> -
> - if (pthread_mutexattr_init (&mutattr) != 0)
> - {
> - puts ("mutexattr_init failed");
> - return 1;
> - }
> -
> - if (pthread_mutexattr_settype (&mutattr, PTHREAD_MUTEX_ERRORCHECK) != 0)
> - {
> - puts ("mutexattr_settype failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_init (&mut, &mutattr) != 0)
> - {
> - puts ("mutex_init failed");
> - return 1;
> - }
> -
> - if (pthread_mutexattr_destroy (&mutattr) != 0)
> - {
> - puts ("mutexattr_destroy failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_lock (&mut) != 0)
> - {
> - puts ("mutex_lock failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_lock (&mut) != EDEADLK)
> - {
> - puts ("2nd mutex_lock did not return EDEADLK");
> - return 1;
> - }
> -
> - struct timespec ts;
> - if (clock_gettime (cl, &ts) != 0)
> - {
> - puts ("clock_gettime failed");
> - return 1;
> - }
> + TEST_COMPARE (pthread_condattr_getclock (&condattr, &cl2), 0);
> + TEST_COMPARE (cl, cl2);
> +
> + TEST_COMPARE (pthread_cond_init (&cond, &condattr), 0);
> + TEST_COMPARE (pthread_condattr_destroy (&condattr), 0);
> +
> + xpthread_mutexattr_init (&mutattr);
> + xpthread_mutexattr_settype (&mutattr, PTHREAD_MUTEX_ERRORCHECK);
> + xpthread_mutex_init (&mut, &mutattr);
> + xpthread_mutexattr_destroy (&mutattr);
> +
> + xpthread_mutex_lock (&mut);
> + TEST_COMPARE (pthread_mutex_lock (&mut), EDEADLK);
> +
> + struct timespec ts_timeout;
> + xclock_gettime (cl, &ts_timeout);
>
> /* Wait one second. */
> - ++ts.tv_sec;
> -
> - int e = pthread_cond_timedwait (&cond, &mut, &ts);
> - if (e == 0)
> - {
> - puts ("cond_timedwait succeeded");
> - return 1;
> - }
> - else if (e != ETIMEDOUT)
> - {
> - puts ("cond_timedwait did not return ETIMEDOUT");
> - return 1;
> - }
> -
> - struct timespec ts2;
> - if (clock_gettime (cl, &ts2) != 0)
> - {
> - puts ("second clock_gettime failed");
> - return 1;
> - }
> -
> - if (ts2.tv_sec < ts.tv_sec
> - || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec < ts.tv_nsec))
> - {
> - puts ("timeout too short");
> - return 1;
> - }
> -
> - if (pthread_mutex_unlock (&mut) != 0)
> - {
> - puts ("mutex_unlock failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_destroy (&mut) != 0)
> - {
> - puts ("mutex_destroy failed");
> - return 1;
> - }
> -
> - if (pthread_cond_destroy (&cond) != 0)
> - {
> - puts ("cond_destroy failed");
> - return 1;
> - }
> + ++ts_timeout.tv_sec;
> +
> + TEST_COMPARE (pthread_cond_timedwait (&cond, &mut, &ts_timeout), ETIMEDOUT);
> + TEST_TIMESPEC_BEFORE_NOW (ts_timeout, cl);
> +
> + xpthread_mutex_unlock (&mut);
> + xpthread_mutex_destroy (&mut);
> + TEST_COMPARE (pthread_cond_destroy (&cond), 0);
>
> return 0;
> }
> @@ -171,12 +79,11 @@ do_test (void)
> {
> #if !defined _POSIX_CLOCK_SELECTION || _POSIX_CLOCK_SELECTION == -1
>
> - puts ("_POSIX_CLOCK_SELECTION not supported, test skipped");
> - return 0;
> + FAIL_UNSUPPORTED ("_POSIX_CLOCK_SELECTION not supported, test skipped");
>
> #else
>
> - int res = run_test (CLOCK_REALTIME);
> + run_test (CLOCK_REALTIME);
>
> # if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
> # if _POSIX_MONOTONIC_CLOCK == 0
> @@ -184,20 +91,16 @@ do_test (void)
> if (e < 0)
> puts ("CLOCK_MONOTONIC not supported");
> else if (e == 0)
> - {
> - puts ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
> - res = 1;
> - }
> + FAIL_RET ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
> else
> # endif
> - res |= run_test (CLOCK_MONOTONIC);
> + run_test (CLOCK_MONOTONIC);
> # else
> puts ("_POSIX_MONOTONIC_CLOCK not defined");
> # endif
>
> - return res;
> + return 0;
> #endif
> }
>
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/support/timespec.h b/support/timespec.h
> new file mode 100644
> index 0000000..d439d62
> --- /dev/null
> +++ b/support/timespec.h
> @@ -0,0 +1,115 @@
> +/* Useful functions for tests that use struct timespec
> + 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef SUPPORT_TIMESPEC_H
> +#define SUPPORT_TIMESPEC_H
> +
> +#include <stdio.h>
> +#include <time.h>
> +#include <support/check.h>
> +#include <support/xtime.h>
> +
> +static inline void
> +timespec_sub (struct timespec *diff, const struct timespec *left,
> + const struct timespec *right)
> +{
> + diff->tv_sec = left->tv_sec - right->tv_sec;
> + diff->tv_nsec = left->tv_nsec - right->tv_nsec;
> +
> + if (diff->tv_nsec < 0)
> + {
> + --diff->tv_sec;
> + diff->tv_nsec += 1000000000;
> + }
> +}
> +
> +static inline void
> +timespec_add (struct timespec *sum, const struct timespec *left,
> + const struct timespec *right)
> +{
> + sum->tv_sec = left->tv_sec + right->tv_sec;
> + sum->tv_nsec = left->tv_nsec + right->tv_nsec;
> +
> + if (sum->tv_nsec >= 1000000000)
> + {
> + ++sum->tv_sec;
> + sum->tv_nsec -= 1000000000;
> + }
> +}
> +
> +/* Check that the timespec on the left represents a time before the
> + time on the right. */
> +#define TEST_TIMESPEC_BEFORE(left, right) \
> + ({ \
> + if ((left).tv_sec > (right).tv_sec \
> + || ((left).tv_sec == (right).tv_sec \
> + && (left).tv_nsec > (right).tv_nsec)) { \
> + const int saved_errno = errno; \
> + support_record_failure(); \
> + struct timespec diff; \
> + timespec_sub(&diff, &(left), &(right)); \
> + printf("%s:%d: %ld.%09lds not before %ld.%09ldus " \
> + "(difference %ld.%09ldus)\n", \
> + __FILE__, __LINE__, \
> + (left).tv_sec, (left).tv_nsec, \
> + (right).tv_sec, (right).tv_nsec, \
> + diff.tv_sec, diff.tv_nsec); \
> + errno = saved_errno; \
> + } \
> + })
> +
> +#define TEST_TIMESPEC_BEFORE_NOW(left, clockid) \
> + ({ \
> + struct timespec now; \
> + const int saved_errno = errno; \
> + xclock_gettime((clockid), &now); \
> + TEST_TIMESPEC_BEFORE((left), now); \
> + errno = saved_errno; \
> + })
> +
> +/* Check that the timespec on the left represents a after before the
> + time on the right. */
> +#define TEST_TIMESPEC_EQUAL_OR_AFTER(left, right) \
> + ({ \
> + if ((left).tv_sec < (right).tv_sec \
> + || ((left).tv_sec == (right).tv_sec \
> + && (left).tv_nsec < (right).tv_nsec)) { \
> + const int saved_errno = errno; \
> + support_record_failure(); \
> + struct timespec diff; \
> + timespec_sub(&diff, &(right), &(left)); \
> + printf("%s:%d: %ld.%09lds not after %ld.%09ldus " \
> + "(difference %ld.%09ldus)\n", \
These should all be "%ld.%09lds" rather than "%ld.09ldus" since the times
are in seconds rather than microseconds.
> + __FILE__, __LINE__, \
> + (left).tv_sec, (left).tv_nsec, \
> + (right).tv_sec, (right).tv_nsec, \
> + diff.tv_sec, diff.tv_nsec); \
> + errno = saved_errno; \
> + } \
> + })
> +
> +#define TEST_TIMESPEC_NOW_OR_AFTER(clockid, right) \
> + ({ \
> + struct timespec now; \
> + const int saved_errno = errno; \
> + xclock_gettime((clockid), &now); \
> + TEST_TIMESPEC_EQUAL_OR_AFTER(now, (right)); \
> + errno = saved_errno; \
> + })
> +
> +#endif /* SUPPORT_TIMESPEC_H */
> --
> git-series 0.9.1
>
On 29/03/2019 01:31, Mike Crowe wrote:
> * support/timespec.h: Create header to provide timespec helper
> functions from sysdeps/pthread/posix-timer.h and macros in the
> style of those in check.h.
>
> * nptl/tst-cond11.c: Use libsupport.
> ---
> ChangeLog | 8 ++-
> nptl/tst-cond11.c | 169 +++++++++-------------------------------------
> support/timespec.h | 115 +++++++++++++++++++++++++++++++-
> 3 files changed, 159 insertions(+), 133 deletions(-)
> create mode 100644 support/timespec.h
>
> diff --git a/ChangeLog b/ChangeLog
> index ff4dabd..5ceb3a7 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,11 @@
> +2019-03-20 Mike Crowe <mac@mcrowe.com>
> +
> + * support/timespec.h: Create header to provide timespec helper
> + functions from sysdeps/pthread/posix-timer.h and macros in the
> + style of those in check.h.
> +
> + * nptl/tst-cond11.c: Use libsupport.
> +
> 2019-03-28 Mike Crowe <mac@mcrowe.com>
>
> * support/xclock_gettime.c (xclock_gettime): New file. Provide
> diff --git a/nptl/tst-cond11.c b/nptl/tst-cond11.c
> index 97a8bd0..3bc4ff4 100644
> --- a/nptl/tst-cond11.c
> +++ b/nptl/tst-cond11.c
> @@ -21,6 +21,10 @@
> #include <stdio.h>
> #include <time.h>
> #include <unistd.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xthread.h>
> +#include <support/xtime.h>
>
>
> #if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0
> @@ -34,132 +38,36 @@ run_test (clockid_t cl)
>
> printf ("clock = %d\n", (int) cl);
>
> - if (pthread_condattr_init (&condattr) != 0)
> - {
> - puts ("condattr_init failed");
> - return 1;
> - }
> -
> - if (pthread_condattr_setclock (&condattr, cl) != 0)
> - {
> - puts ("condattr_setclock failed");
> - return 1;
> - }
> + TEST_COMPARE (pthread_condattr_init (&condattr), 0);
> + TEST_COMPARE (pthread_condattr_setclock (&condattr, cl), 0);
Ok.
>
> clockid_t cl2;
> - if (pthread_condattr_getclock (&condattr, &cl2) != 0)
> - {
> - puts ("condattr_getclock failed");
> - return 1;
> - }
> - if (cl != cl2)
> - {
> - printf ("condattr_getclock returned wrong value: %d, expected %d\n",
> - (int) cl2, (int) cl);
> - return 1;
> - }
> -
> - if (pthread_cond_init (&cond, &condattr) != 0)
> - {
> - puts ("cond_init failed");
> - return 1;
> - }
> -
> - if (pthread_condattr_destroy (&condattr) != 0)
> - {
> - puts ("condattr_destroy failed");
> - return 1;
> - }
> -
> - if (pthread_mutexattr_init (&mutattr) != 0)
> - {
> - puts ("mutexattr_init failed");
> - return 1;
> - }
> -
> - if (pthread_mutexattr_settype (&mutattr, PTHREAD_MUTEX_ERRORCHECK) != 0)
> - {
> - puts ("mutexattr_settype failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_init (&mut, &mutattr) != 0)
> - {
> - puts ("mutex_init failed");
> - return 1;
> - }
> -
> - if (pthread_mutexattr_destroy (&mutattr) != 0)
> - {
> - puts ("mutexattr_destroy failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_lock (&mut) != 0)
> - {
> - puts ("mutex_lock failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_lock (&mut) != EDEADLK)
> - {
> - puts ("2nd mutex_lock did not return EDEADLK");
> - return 1;
> - }
> -
> - struct timespec ts;
> - if (clock_gettime (cl, &ts) != 0)
> - {
> - puts ("clock_gettime failed");
> - return 1;
> - }
> + TEST_COMPARE (pthread_condattr_getclock (&condattr, &cl2), 0);
> + TEST_COMPARE (cl, cl2);
> +
> + TEST_COMPARE (pthread_cond_init (&cond, &condattr), 0);
> + TEST_COMPARE (pthread_condattr_destroy (&condattr), 0);
> +
> + xpthread_mutexattr_init (&mutattr);
> + xpthread_mutexattr_settype (&mutattr, PTHREAD_MUTEX_ERRORCHECK);
> + xpthread_mutex_init (&mut, &mutattr);
> + xpthread_mutexattr_destroy (&mutattr);
> +
> + xpthread_mutex_lock (&mut);
> + TEST_COMPARE (pthread_mutex_lock (&mut), EDEADLK);
> +
> + struct timespec ts_timeout;
> + xclock_gettime (cl, &ts_timeout);
>
> /* Wait one second. */
> - ++ts.tv_sec;
> -
> - int e = pthread_cond_timedwait (&cond, &mut, &ts);
> - if (e == 0)
> - {
> - puts ("cond_timedwait succeeded");
> - return 1;
> - }
> - else if (e != ETIMEDOUT)
> - {
> - puts ("cond_timedwait did not return ETIMEDOUT");
> - return 1;
> - }
> -
> - struct timespec ts2;
> - if (clock_gettime (cl, &ts2) != 0)
> - {
> - puts ("second clock_gettime failed");
> - return 1;
> - }
> -
> - if (ts2.tv_sec < ts.tv_sec
> - || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec < ts.tv_nsec))
> - {
> - puts ("timeout too short");
> - return 1;
> - }
> -
> - if (pthread_mutex_unlock (&mut) != 0)
> - {
> - puts ("mutex_unlock failed");
> - return 1;
> - }
> -
> - if (pthread_mutex_destroy (&mut) != 0)
> - {
> - puts ("mutex_destroy failed");
> - return 1;
> - }
> -
> - if (pthread_cond_destroy (&cond) != 0)
> - {
> - puts ("cond_destroy failed");
> - return 1;
> - }
> + ++ts_timeout.tv_sec;
> +
> + TEST_COMPARE (pthread_cond_timedwait (&cond, &mut, &ts_timeout), ETIMEDOUT);
> + TEST_TIMESPEC_BEFORE_NOW (ts_timeout, cl);
> +
> + xpthread_mutex_unlock (&mut);
> + xpthread_mutex_destroy (&mut);
> + TEST_COMPARE (pthread_cond_destroy (&cond), 0);
Ok.
>
> return 0;
> }
> @@ -171,12 +79,11 @@ do_test (void)
> {
> #if !defined _POSIX_CLOCK_SELECTION || _POSIX_CLOCK_SELECTION == -1
>
> - puts ("_POSIX_CLOCK_SELECTION not supported, test skipped");
> - return 0;
> + FAIL_UNSUPPORTED ("_POSIX_CLOCK_SELECTION not supported, test skipped");
Ok.
>
> #else
>
> - int res = run_test (CLOCK_REALTIME);
> + run_test (CLOCK_REALTIME);
Ok.
>
> # if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
> # if _POSIX_MONOTONIC_CLOCK == 0
> @@ -184,20 +91,16 @@ do_test (void)
> if (e < 0)
> puts ("CLOCK_MONOTONIC not supported");
> else if (e == 0)
> - {
> - puts ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
> - res = 1;
> - }
> + FAIL_RET ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
Ok.
> else
> # endif
> - res |= run_test (CLOCK_MONOTONIC);
> + run_test (CLOCK_MONOTONIC);
> # else
> puts ("_POSIX_MONOTONIC_CLOCK not defined");
> # endif
>
> - return res;
> + return 0;
> #endif
> }
Ok.
>
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/support/timespec.h b/support/timespec.h
> new file mode 100644
> index 0000000..d439d62
> --- /dev/null
> +++ b/support/timespec.h
> @@ -0,0 +1,115 @@
> +/* Useful functions for tests that use struct timespec
Missing end period.
> + 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef SUPPORT_TIMESPEC_H
> +#define SUPPORT_TIMESPEC_H
> +
> +#include <stdio.h>
> +#include <time.h>
> +#include <support/check.h>
> +#include <support/xtime.h>
> +
> +static inline void
> +timespec_sub (struct timespec *diff, const struct timespec *left,
> + const struct timespec *right)
> +{
> + diff->tv_sec = left->tv_sec - right->tv_sec;
> + diff->tv_nsec = left->tv_nsec - right->tv_nsec;
> +
> + if (diff->tv_nsec < 0)
> + {
> + --diff->tv_sec;
> + diff->tv_nsec += 1000000000;
> + }
> +}
> +
I think we should handle overflow in such tests and assuming always signed
time_t we can use a simplified version of gnulib (if I get everything right):
---
void
timespec_subtract (struct timespec *r, const struct timespec *a,
const struct timespec *b)
{
time_t rs = a->tv_sec;
time_t bs = b->tv_sec;
long int ns = a->tv_nsec - b->tv_nsec;
long int rns = ns;
if (ns < 0)
{
rns = ns + TIMESPEC_HZ;
if (bs < TYPE_MAXIMUM (time_t))
bs++;
else
{
rs = TYPE_MINIMUM (time_t);
rns = 0;
}
}
if (!__builtin_sub_overflow (rs, bs, &rs))
rs -= bs;
else
{
if (rs < 0)
{
rs = TYPE_MINIMUM (time_t);
rns = 0;
}
else
{
rs = TYPE_MAXIMUM (time_t);
rns = TIMESPEC_HZ - 1;
}
}
*r = (struct timespec) { ns, rns };
}
---
> +static inline void
> +timespec_add (struct timespec *sum, const struct timespec *left,
> + const struct timespec *right)
> +{
> + sum->tv_sec = left->tv_sec + right->tv_sec;
> + sum->tv_nsec = left->tv_nsec + right->tv_nsec;
> +
> + if (sum->tv_nsec >= 1000000000)
> + {
> + ++sum->tv_sec;
> + sum->tv_nsec -= 1000000000;
> + }
> +}
Same as before.
> +
> +/* Check that the timespec on the left represents a time before the
> + time on the right. */
> +#define TEST_TIMESPEC_BEFORE(left, right) \
> + ({ \
> + if ((left).tv_sec > (right).tv_sec \
> + || ((left).tv_sec == (right).tv_sec \
> + && (left).tv_nsec > (right).tv_nsec)) { \
> + const int saved_errno = errno; \
> + support_record_failure(); \
> + struct timespec diff; \
> + timespec_sub(&diff, &(left), &(right)); \
Space after timespec_sub.
> + printf("%s:%d: %ld.%09lds not before %ld.%09ldus " \
Same for printf.
> + "(difference %ld.%09ldus)\n", \
> + __FILE__, __LINE__, \
> + (left).tv_sec, (left).tv_nsec, \
> + (right).tv_sec, (right).tv_nsec, \
> + diff.tv_sec, diff.tv_nsec); \
> + errno = saved_errno; \
> + } \
> + })
> +
> +#define TEST_TIMESPEC_BEFORE_NOW(left, clockid) \
> + ({ \
> + struct timespec now; \
> + const int saved_errno = errno; \
> + xclock_gettime((clockid), &now); \
Ditto.
> + TEST_TIMESPEC_BEFORE((left), now); \
Ditto.
> + errno = saved_errno; \
> + })
> +
> +/* Check that the timespec on the left represents a after before the
> + time on the right. */
> +#define TEST_TIMESPEC_EQUAL_OR_AFTER(left, right) \
> + ({ \
> + if ((left).tv_sec < (right).tv_sec \
> + || ((left).tv_sec == (right).tv_sec \
> + && (left).tv_nsec < (right).tv_nsec)) { \
> + const int saved_errno = errno; \
> + support_record_failure(); \
> + struct timespec diff; \
> + timespec_sub(&diff, &(right), &(left)); \
> + printf("%s:%d: %ld.%09lds not after %ld.%09ldus " \
> + "(difference %ld.%09ldus)\n", \
> + __FILE__, __LINE__, \
> + (left).tv_sec, (left).tv_nsec, \
> + (right).tv_sec, (right).tv_nsec, \
> + diff.tv_sec, diff.tv_nsec); \
> + errno = saved_errno; \
> + } \
> + })
> +
> +#define TEST_TIMESPEC_NOW_OR_AFTER(clockid, right) \
> + ({ \
> + struct timespec now; \
> + const int saved_errno = errno; \
> + xclock_gettime((clockid), &now); \
> + TEST_TIMESPEC_EQUAL_OR_AFTER(now, (right)); \
> + errno = saved_errno; \
> + })
> +
> +#endif /* SUPPORT_TIMESPEC_H */
>
On Wednesday 03 April 2019 at 09:18:56 +0700, Adhemerval Zanella wrote:
> On 29/03/2019 01:31, Mike Crowe wrote:
> > diff --git a/support/timespec.h b/support/timespec.h
> > new file mode 100644
> > index 0000000..d439d62
> > --- /dev/null
> > +++ b/support/timespec.h
> > @@ -0,0 +1,115 @@
[...]
> > +static inline void
> > +timespec_sub (struct timespec *diff, const struct timespec *left,
> > + const struct timespec *right)
> > +{
> > + diff->tv_sec = left->tv_sec - right->tv_sec;
> > + diff->tv_nsec = left->tv_nsec - right->tv_nsec;
> > +
> > + if (diff->tv_nsec < 0)
> > + {
> > + --diff->tv_sec;
> > + diff->tv_nsec += 1000000000;
> > + }
> > +}
> > +
>
> I think we should handle overflow in such tests and assuming always signed
> time_t we can use a simplified version of gnulib (if I get everything right):
>
> ---
> void
> timespec_subtract (struct timespec *r, const struct timespec *a,
> const struct timespec *b)
> {
> time_t rs = a->tv_sec;
> time_t bs = b->tv_sec;
> long int ns = a->tv_nsec - b->tv_nsec;
> long int rns = ns;
>
> if (ns < 0)
> {
> rns = ns + TIMESPEC_HZ;
> if (bs < TYPE_MAXIMUM (time_t))
> bs++;
> else
> {
> rs = TYPE_MINIMUM (time_t);
> rns = 0;
> }
> }
>
> if (!__builtin_sub_overflow (rs, bs, &rs))
> rs -= bs;
> else
> {
> if (rs < 0)
> {
> rs = TYPE_MINIMUM (time_t);
> rns = 0;
> }
> else
> {
> rs = TYPE_MAXIMUM (time_t);
> rns = TIMESPEC_HZ - 1;
> }
> }
>
> *r = (struct timespec) { ns, rns };
> }
That seems to rely on macros from include/intprops.h anyway, so is there
any disadvantage in just using the gnulib version as it stands (along with
a suitable definition of TIMESPEC_HZ)?
But, having said that, this code from gnulib is GPLv3 (and it always has
been), whereas glibc is LGPLv2.1. Surely this means that we can't just copy
the code across?
Mike.
On 4/3/19 1:55 PM, Mike Crowe wrote:
> That seems to rely on macros from include/intprops.h anyway, so is there
> any disadvantage in just using the gnulib version as it stands (along with
> a suitable definition of TIMESPEC_HZ)?
>
> But, having said that, this code from gnulib is GPLv3 (and it always has
> been), whereas glibc is LGPLv2.1. Surely this means that we can't just copy
> the code across?
That shouldn't be a problem, as the relevant Gnulib module is LGPLed not
GPLed. The Gnulib tool for extracting the code from Gnulib relicenses
the code automatically to match the package the code is used in.
On Wednesday 03 April 2019 at 16:35:23 -0700, Paul Eggert wrote:
> On 4/3/19 1:55 PM, Mike Crowe wrote:
> > That seems to rely on macros from include/intprops.h anyway, so is there
> > any disadvantage in just using the gnulib version as it stands (along with
> > a suitable definition of TIMESPEC_HZ)?
> >
> > But, having said that, this code from gnulib is GPLv3 (and it always has
> > been), whereas glibc is LGPLv2.1. Surely this means that we can't just copy
> > the code across?
>
> That shouldn't be a problem, as the relevant Gnulib module is LGPLed not
> GPLed. The Gnulib tool for extracting the code from Gnulib relicenses
> the code automatically to match the package the code is used in.
Since you're the author I'm wary of arguing, but in current gnulib master
modules/timespec-add contains:
License:
GPL
and when I run gnulib-tool the licence in "installed" source files is
GPLv3+ :(
Mike.
On 4/4/19 5:45 AM, Mike Crowe wrote:
> Since you're the author I'm wary of arguing, but in current gnulib master
> modules/timespec-add contains:
>
> License:
> GPL
Ah, you're looking at the wrong module. Try modules/timespec.
Besides, we routinely change Gnulib library code from GPL to LGPL if the
code is reasonable to use in library code, which is the case here.
On Thursday 04 April 2019 at 14:48:12 -0700, Paul Eggert wrote:
> On 4/4/19 5:45 AM, Mike Crowe wrote:
> > Since you're the author I'm wary of arguing, but in current gnulib master
> > modules/timespec-add contains:
> >
> > License:
> > GPL
>
> Ah, you're looking at the wrong module. Try modules/timespec.
Perhaps I'm being completely inept, but using gnulib-tool to import
modules/timespec just seems to bring in timespec.h (which includes only
declarations of timespec_add and timespec_sub) and timespec.c (which
contains no code.) The actual implementations in timespec-add.c and
timespec-sub.c are not imported.
> Besides, we routinely change Gnulib library code from GPL to LGPL if the
> code is reasonable to use in library code, which is the case here.
I shall proceed on the basis that the implementations in gnulib's
timespec-add.c and timespec-sub.c are safe to bring into glibc and be
relicensed as LGPLv2.1+.
Thanks for your help.
Mike.
@@ -1,3 +1,11 @@
+2019-03-20 Mike Crowe <mac@mcrowe.com>
+
+ * support/timespec.h: Create header to provide timespec helper
+ functions from sysdeps/pthread/posix-timer.h and macros in the
+ style of those in check.h.
+
+ * nptl/tst-cond11.c: Use libsupport.
+
2019-03-28 Mike Crowe <mac@mcrowe.com>
* support/xclock_gettime.c (xclock_gettime): New file. Provide
@@ -21,6 +21,10 @@
#include <stdio.h>
#include <time.h>
#include <unistd.h>
+#include <support/check.h>
+#include <support/timespec.h>
+#include <support/xthread.h>
+#include <support/xtime.h>
#if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0
@@ -34,132 +38,36 @@ run_test (clockid_t cl)
printf ("clock = %d\n", (int) cl);
- if (pthread_condattr_init (&condattr) != 0)
- {
- puts ("condattr_init failed");
- return 1;
- }
-
- if (pthread_condattr_setclock (&condattr, cl) != 0)
- {
- puts ("condattr_setclock failed");
- return 1;
- }
+ TEST_COMPARE (pthread_condattr_init (&condattr), 0);
+ TEST_COMPARE (pthread_condattr_setclock (&condattr, cl), 0);
clockid_t cl2;
- if (pthread_condattr_getclock (&condattr, &cl2) != 0)
- {
- puts ("condattr_getclock failed");
- return 1;
- }
- if (cl != cl2)
- {
- printf ("condattr_getclock returned wrong value: %d, expected %d\n",
- (int) cl2, (int) cl);
- return 1;
- }
-
- if (pthread_cond_init (&cond, &condattr) != 0)
- {
- puts ("cond_init failed");
- return 1;
- }
-
- if (pthread_condattr_destroy (&condattr) != 0)
- {
- puts ("condattr_destroy failed");
- return 1;
- }
-
- if (pthread_mutexattr_init (&mutattr) != 0)
- {
- puts ("mutexattr_init failed");
- return 1;
- }
-
- if (pthread_mutexattr_settype (&mutattr, PTHREAD_MUTEX_ERRORCHECK) != 0)
- {
- puts ("mutexattr_settype failed");
- return 1;
- }
-
- if (pthread_mutex_init (&mut, &mutattr) != 0)
- {
- puts ("mutex_init failed");
- return 1;
- }
-
- if (pthread_mutexattr_destroy (&mutattr) != 0)
- {
- puts ("mutexattr_destroy failed");
- return 1;
- }
-
- if (pthread_mutex_lock (&mut) != 0)
- {
- puts ("mutex_lock failed");
- return 1;
- }
-
- if (pthread_mutex_lock (&mut) != EDEADLK)
- {
- puts ("2nd mutex_lock did not return EDEADLK");
- return 1;
- }
-
- struct timespec ts;
- if (clock_gettime (cl, &ts) != 0)
- {
- puts ("clock_gettime failed");
- return 1;
- }
+ TEST_COMPARE (pthread_condattr_getclock (&condattr, &cl2), 0);
+ TEST_COMPARE (cl, cl2);
+
+ TEST_COMPARE (pthread_cond_init (&cond, &condattr), 0);
+ TEST_COMPARE (pthread_condattr_destroy (&condattr), 0);
+
+ xpthread_mutexattr_init (&mutattr);
+ xpthread_mutexattr_settype (&mutattr, PTHREAD_MUTEX_ERRORCHECK);
+ xpthread_mutex_init (&mut, &mutattr);
+ xpthread_mutexattr_destroy (&mutattr);
+
+ xpthread_mutex_lock (&mut);
+ TEST_COMPARE (pthread_mutex_lock (&mut), EDEADLK);
+
+ struct timespec ts_timeout;
+ xclock_gettime (cl, &ts_timeout);
/* Wait one second. */
- ++ts.tv_sec;
-
- int e = pthread_cond_timedwait (&cond, &mut, &ts);
- if (e == 0)
- {
- puts ("cond_timedwait succeeded");
- return 1;
- }
- else if (e != ETIMEDOUT)
- {
- puts ("cond_timedwait did not return ETIMEDOUT");
- return 1;
- }
-
- struct timespec ts2;
- if (clock_gettime (cl, &ts2) != 0)
- {
- puts ("second clock_gettime failed");
- return 1;
- }
-
- if (ts2.tv_sec < ts.tv_sec
- || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec < ts.tv_nsec))
- {
- puts ("timeout too short");
- return 1;
- }
-
- if (pthread_mutex_unlock (&mut) != 0)
- {
- puts ("mutex_unlock failed");
- return 1;
- }
-
- if (pthread_mutex_destroy (&mut) != 0)
- {
- puts ("mutex_destroy failed");
- return 1;
- }
-
- if (pthread_cond_destroy (&cond) != 0)
- {
- puts ("cond_destroy failed");
- return 1;
- }
+ ++ts_timeout.tv_sec;
+
+ TEST_COMPARE (pthread_cond_timedwait (&cond, &mut, &ts_timeout), ETIMEDOUT);
+ TEST_TIMESPEC_BEFORE_NOW (ts_timeout, cl);
+
+ xpthread_mutex_unlock (&mut);
+ xpthread_mutex_destroy (&mut);
+ TEST_COMPARE (pthread_cond_destroy (&cond), 0);
return 0;
}
@@ -171,12 +79,11 @@ do_test (void)
{
#if !defined _POSIX_CLOCK_SELECTION || _POSIX_CLOCK_SELECTION == -1
- puts ("_POSIX_CLOCK_SELECTION not supported, test skipped");
- return 0;
+ FAIL_UNSUPPORTED ("_POSIX_CLOCK_SELECTION not supported, test skipped");
#else
- int res = run_test (CLOCK_REALTIME);
+ run_test (CLOCK_REALTIME);
# if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
# if _POSIX_MONOTONIC_CLOCK == 0
@@ -184,20 +91,16 @@ do_test (void)
if (e < 0)
puts ("CLOCK_MONOTONIC not supported");
else if (e == 0)
- {
- puts ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
- res = 1;
- }
+ FAIL_RET ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
else
# endif
- res |= run_test (CLOCK_MONOTONIC);
+ run_test (CLOCK_MONOTONIC);
# else
puts ("_POSIX_MONOTONIC_CLOCK not defined");
# endif
- return res;
+ return 0;
#endif
}
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
new file mode 100644
@@ -0,0 +1,115 @@
+/* Useful functions for tests that use struct timespec
+ 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
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef SUPPORT_TIMESPEC_H
+#define SUPPORT_TIMESPEC_H
+
+#include <stdio.h>
+#include <time.h>
+#include <support/check.h>
+#include <support/xtime.h>
+
+static inline void
+timespec_sub (struct timespec *diff, const struct timespec *left,
+ const struct timespec *right)
+{
+ diff->tv_sec = left->tv_sec - right->tv_sec;
+ diff->tv_nsec = left->tv_nsec - right->tv_nsec;
+
+ if (diff->tv_nsec < 0)
+ {
+ --diff->tv_sec;
+ diff->tv_nsec += 1000000000;
+ }
+}
+
+static inline void
+timespec_add (struct timespec *sum, const struct timespec *left,
+ const struct timespec *right)
+{
+ sum->tv_sec = left->tv_sec + right->tv_sec;
+ sum->tv_nsec = left->tv_nsec + right->tv_nsec;
+
+ if (sum->tv_nsec >= 1000000000)
+ {
+ ++sum->tv_sec;
+ sum->tv_nsec -= 1000000000;
+ }
+}
+
+/* Check that the timespec on the left represents a time before the
+ time on the right. */
+#define TEST_TIMESPEC_BEFORE(left, right) \
+ ({ \
+ if ((left).tv_sec > (right).tv_sec \
+ || ((left).tv_sec == (right).tv_sec \
+ && (left).tv_nsec > (right).tv_nsec)) { \
+ const int saved_errno = errno; \
+ support_record_failure(); \
+ struct timespec diff; \
+ timespec_sub(&diff, &(left), &(right)); \
+ printf("%s:%d: %ld.%09lds not before %ld.%09ldus " \
+ "(difference %ld.%09ldus)\n", \
+ __FILE__, __LINE__, \
+ (left).tv_sec, (left).tv_nsec, \
+ (right).tv_sec, (right).tv_nsec, \
+ diff.tv_sec, diff.tv_nsec); \
+ errno = saved_errno; \
+ } \
+ })
+
+#define TEST_TIMESPEC_BEFORE_NOW(left, clockid) \
+ ({ \
+ struct timespec now; \
+ const int saved_errno = errno; \
+ xclock_gettime((clockid), &now); \
+ TEST_TIMESPEC_BEFORE((left), now); \
+ errno = saved_errno; \
+ })
+
+/* Check that the timespec on the left represents a after before the
+ time on the right. */
+#define TEST_TIMESPEC_EQUAL_OR_AFTER(left, right) \
+ ({ \
+ if ((left).tv_sec < (right).tv_sec \
+ || ((left).tv_sec == (right).tv_sec \
+ && (left).tv_nsec < (right).tv_nsec)) { \
+ const int saved_errno = errno; \
+ support_record_failure(); \
+ struct timespec diff; \
+ timespec_sub(&diff, &(right), &(left)); \
+ printf("%s:%d: %ld.%09lds not after %ld.%09ldus " \
+ "(difference %ld.%09ldus)\n", \
+ __FILE__, __LINE__, \
+ (left).tv_sec, (left).tv_nsec, \
+ (right).tv_sec, (right).tv_nsec, \
+ diff.tv_sec, diff.tv_nsec); \
+ errno = saved_errno; \
+ } \
+ })
+
+#define TEST_TIMESPEC_NOW_OR_AFTER(clockid, right) \
+ ({ \
+ struct timespec now; \
+ const int saved_errno = errno; \
+ xclock_gettime((clockid), &now); \
+ TEST_TIMESPEC_EQUAL_OR_AFTER(now, (right)); \
+ errno = saved_errno; \
+ })
+
+#endif /* SUPPORT_TIMESPEC_H */