[PATCH v2 0/6] Add new helper functions+macros to libsupport and use them in some nptl tests

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 0/6] Add new helper functions+macros to libsupport and use them in some nptl tests

Mike Crowe
In preparation for adding support for the _clockwait family of
functions, modify various tests to:

1. use struct timespec rather than struct timeval

2. use libsupport.

This requires adding a new support/timespec.h and support/xtime.h
containing helper functions and macros along with timespec_sub and
timespec_add implementations from gnulib.

Thanks to Adhemerval Zanella for providing lots of help and advice
with these patches. They haven't been reviewed yet, so the flaws are
all my own.

Changes since v1[1]:

* Many whitespace fixes.

* The check macros in timespec.h now call out-of-line functions rather
  than containing the code themselves.

* Take better timespec_add and timespec_sub implementations from
  gnulib.

* Use xclock_gettime in more places.

* Split addition of support/timespec.h into a separate patch.

* Combine two separate rwlock patches into one to hopefully make
  review easier.

* Use even more x* wrapper functions.

[1] https://lwn.net/ml/libc-alpha/cover.13c2d68f411f9010956e8e52edfbe168964e1e5c.1553797867.git-series.mac%40mcrowe.com/

Mike Crowe (6):
  support: Add xclock_gettime
  support: Add timespec-related helper functions
  nptl: Convert tst-cond11.c to use libsupport
  nptl: Use recent additions to libsupport in tst-sem5
  nptl: Convert some rwlock tests to use libsupport
  nptl/tst-abstime: Use libsupport

 ChangeLog                |  51 ++++++++++++-
 nptl/tst-abstime.c       |  70 ++++-------------
 nptl/tst-cond11.c        | 169 ++++++++--------------------------------
 nptl/tst-rwlock14.c      | 122 ++++-------------------------
 nptl/tst-rwlock6.c       | 146 ++++++-----------------------------
 nptl/tst-rwlock7.c       | 119 ++++++----------------------
 nptl/tst-rwlock9.c       |  96 +++++------------------
 nptl/tst-sem5.c          |  23 +----
 support/Makefile         |   4 +-
 support/README           |   6 +-
 support/timespec-add.c   |  75 ++++++++++++++++++-
 support/timespec-sub.c   |  75 ++++++++++++++++++-
 support/timespec.c       |  63 +++++++++++++++-
 support/timespec.h       |  79 +++++++++++++++++++-
 support/xclock_gettime.c |  29 +++++++-
 support/xtime.h          |  33 ++++++++-
 16 files changed, 571 insertions(+), 589 deletions(-)
 create mode 100644 support/timespec-add.c
 create mode 100644 support/timespec-sub.c
 create mode 100644 support/timespec.c
 create mode 100644 support/timespec.h
 create mode 100644 support/xclock_gettime.c
 create mode 100644 support/xtime.h

base-commit: 94e358f6d490650c714edb1ffc3a52f56ffe086e
--
git-series 0.9.1
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/6] support: Add xclock_gettime

Mike Crowe
* support/xclock_gettime.c (xclock_gettime): New file. Provide
        clock_gettime wrapper for use in tests that fails the test rather
        than returning failure.

        * support/xtime.h: New file to declare xclock_gettime.

        * support/Makefile: Add xclock_gettime.c.

        * support/README: Mention xtime.h.
---
 ChangeLog                | 12 ++++++++++++
 support/Makefile         |  1 +
 support/README           |  1 +
 support/xclock_gettime.c | 29 +++++++++++++++++++++++++++++
 support/xtime.h          | 33 +++++++++++++++++++++++++++++++++
 5 files changed, 76 insertions(+)
 create mode 100644 support/xclock_gettime.c
 create mode 100644 support/xtime.h

diff --git a/ChangeLog b/ChangeLog
index 5ad8875..8c26806 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2019-04-06  Mike Crowe  <[hidden email]>
+
+ * support/xclock_gettime.c (xclock_gettime): New file. Provide
+ clock_gettime wrapper for use in tests that fails the test rather
+ than returning failure.
+
+ * support/xtime.h: New file to declare xclock_gettime.
+
+ * support/Makefile: Add xclock_gettime.c.
+
+ * support/README: Mention xtime.h.
+
 2019-04-05  Anton Youdkevitch <[hidden email]>
 
  * sysdeps/aarch64/multiarch/memcpy_thunderx2.S: Cleanup branching
diff --git a/support/Makefile b/support/Makefile
index f173565..1d37f70 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -77,6 +77,7 @@ libsupport-routines = \
   xbind \
   xcalloc \
   xchroot \
+  xclock_gettime \
   xclose \
   xconnect \
   xcopy_file_range \
diff --git a/support/README b/support/README
index 476cfcd..d82f472 100644
--- a/support/README
+++ b/support/README
@@ -10,6 +10,7 @@ error.  They are declared in these header files:
 * support.h
 * xsignal.h
 * xthread.h
+* xtime.h
 
 In general, new wrappers should be added to support.h if possible.
 However, support.h must remain fully compatible with C90 and therefore
diff --git a/support/xclock_gettime.c b/support/xclock_gettime.c
new file mode 100644
index 0000000..91464fe
--- /dev/null
+++ b/support/xclock_gettime.c
@@ -0,0 +1,29 @@
+/* clock_gettime with error checking.
+   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/>.  */
+
+#include <support/xtime.h>
+#include <support/xthread.h>
+
+
+void
+xclock_gettime (clockid_t clockid,
+                struct timespec *ts)
+{
+  xpthread_check_return
+    ("clock_gettime", clock_gettime (clockid, ts));
+}
diff --git a/support/xtime.h b/support/xtime.h
new file mode 100644
index 0000000..68af1a5
--- /dev/null
+++ b/support/xtime.h
@@ -0,0 +1,33 @@
+/* Support functionality for using time.
+   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_TIME_H
+#define SUPPORT_TIME_H
+
+#include <time.h>
+
+__BEGIN_DECLS
+
+/* The following functions call the corresponding libc functions and
+   terminate the process on error.  */
+
+void xclock_gettime (clockid_t clock, struct timespec *ts);
+
+__END_DECLS
+
+#endif /* SUPPORT_TIME_H */
--
git-series 0.9.1
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 2/6] support: Add timespec-related helper functions

Mike Crowe
In reply to this post by Mike Crowe
* support/timespec.h: New file. Provide timespec helper functions
        along with macros in the style of those in check.h.

        * support/timespec.c: New file. Implement check functions declared
        in support/timespec.h.

        * support/timespec-add.c: New file from gnulib containing
        timespec_add implementation that handles overflow.

        * support/timespec-sub.c: New file from gnulib containing
        timespec_sub implementation that handles overflow.

        * support/README: Mention timespec.h.
---
 ChangeLog              | 16 +++++++++-
 support/Makefile       |  3 ++-
 support/README         |  5 +++-
 support/timespec-add.c | 75 +++++++++++++++++++++++++++++++++++++++++-
 support/timespec-sub.c | 75 +++++++++++++++++++++++++++++++++++++++++-
 support/timespec.c     | 63 ++++++++++++++++++++++++++++++++++-
 support/timespec.h     | 79 +++++++++++++++++++++++++++++++++++++++++++-
 7 files changed, 316 insertions(+)
 create mode 100644 support/timespec-add.c
 create mode 100644 support/timespec-sub.c
 create mode 100644 support/timespec.c
 create mode 100644 support/timespec.h

diff --git a/ChangeLog b/ChangeLog
index 8c26806..49dc84e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2019-04-06  Mike Crowe  <[hidden email]>
 
+ * support/timespec.h: New file. Provide timespec helper functions
+ along with macros in the style of those in check.h.
+
+ * support/timespec.c: New file. Implement check functions declared
+ in support/timespec.h.
+
+ * support/timespec-add.c: New file from gnulib containing
+ timespec_add implementation that handles overflow.
+
+ * support/timespec-sub.c: New file from gnulib containing
+ timespec_sub implementation that handles overflow.
+
+ * support/README: Mention timespec.h.
+
+2019-04-06  Mike Crowe  <[hidden email]>
+
  * support/xclock_gettime.c (xclock_gettime): New file. Provide
  clock_gettime wrapper for use in tests that fails the test rather
  than returning failure.
diff --git a/support/Makefile b/support/Makefile
index 1d37f70..94a1416 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -70,6 +70,9 @@ libsupport-routines = \
   support_test_main \
   support_test_verify_impl \
   temp_file \
+  timespec \
+  timespec-add \
+  timespec-sub \
   write_message \
   xaccept \
   xaccept4 \
diff --git a/support/README b/support/README
index d82f472..e33067e 100644
--- a/support/README
+++ b/support/README
@@ -28,3 +28,8 @@ header files provide related declarations:
 * check.h
 * temp_file.h
 * test-driver.h
+
+For tests that make use of struct timespec, the following header file
+contains additional macros and helper functions:
+
+* timespec.h
diff --git a/support/timespec-add.c b/support/timespec-add.c
new file mode 100644
index 0000000..523eac8
--- /dev/null
+++ b/support/timespec-add.c
@@ -0,0 +1,75 @@
+/* Add two struct timespec values.
+
+   Copyright (C) 2011-2019 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library, although it originally came
+   from gnulib.
+
+   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/>.  */
+
+/* Written by Paul Eggert.  */
+
+/* Return the sum of two timespec values A and B.  On overflow, return
+   an extremal value.  This assumes 0 <= tv_nsec < TIMESPEC_HZ.  */
+
+#include <config.h>
+#include "timespec.h"
+
+#include "intprops.h"
+
+struct timespec
+timespec_add (struct timespec a, struct timespec b)
+{
+  time_t rs = a.tv_sec;
+  time_t bs = b.tv_sec;
+  int ns = a.tv_nsec + b.tv_nsec;
+  int nsd = ns - TIMESPEC_HZ;
+  int rns = ns;
+  time_t tmin = TYPE_MINIMUM (time_t);
+  time_t tmax = TYPE_MAXIMUM (time_t);
+
+  if (0 <= nsd)
+    {
+      rns = nsd;
+      if (bs < tmax)
+        bs++;
+      else if (rs < 0)
+        rs++;
+      else
+        goto high_overflow;
+    }
+
+  /* INT_ADD_WRAPV is not appropriate since time_t might be unsigned.
+     In theory time_t might be narrower than int, so plain
+     INT_ADD_OVERFLOW does not suffice.  */
+  if (! INT_ADD_OVERFLOW (rs, bs) && tmin <= rs + bs && rs + bs <= tmax)
+    rs += bs;
+  else
+    {
+      if (rs < 0)
+        {
+          rs = tmin;
+          rns = 0;
+        }
+      else
+        {
+        high_overflow:
+          rs = tmax;
+          rns = TIMESPEC_HZ - 1;
+        }
+    }
+
+  return make_timespec (rs, rns);
+}
diff --git a/support/timespec-sub.c b/support/timespec-sub.c
new file mode 100644
index 0000000..7e67e87
--- /dev/null
+++ b/support/timespec-sub.c
@@ -0,0 +1,75 @@
+/* Subtract two struct timespec values.
+
+   Copyright (C) 2011-2019 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library, although it originally came
+   from gnulib.
+
+   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/>.  */
+
+/* Written by Paul Eggert.  */
+
+/* Return the difference between two timespec values A and B.  On
+   overflow, return an extremal value.  This assumes 0 <= tv_nsec <
+   TIMESPEC_HZ.  */
+
+#include <config.h>
+#include "timespec.h"
+
+#include "intprops.h"
+
+struct timespec
+timespec_sub (struct timespec a, struct timespec b)
+{
+  time_t rs = a.tv_sec;
+  time_t bs = b.tv_sec;
+  int ns = a.tv_nsec - b.tv_nsec;
+  int rns = ns;
+  time_t tmin = TYPE_MINIMUM (time_t);
+  time_t tmax = TYPE_MAXIMUM (time_t);
+
+  if (ns < 0)
+    {
+      rns = ns + TIMESPEC_HZ;
+      if (bs < tmax)
+        bs++;
+      else if (- TYPE_SIGNED (time_t) < rs)
+        rs--;
+      else
+        goto low_overflow;
+    }
+
+  /* INT_SUBTRACT_WRAPV is not appropriate since time_t might be unsigned.
+     In theory time_t might be narrower than int, so plain
+     INT_SUBTRACT_OVERFLOW does not suffice.  */
+  if (! INT_SUBTRACT_OVERFLOW (rs, bs) && tmin <= rs - bs && rs - bs <= tmax)
+    rs -= bs;
+  else
+    {
+      if (rs < 0)
+        {
+        low_overflow:
+          rs = tmin;
+          rns = 0;
+        }
+      else
+        {
+          rs = tmax;
+          rns = TIMESPEC_HZ - 1;
+        }
+    }
+
+  return make_timespec (rs, rns);
+}
diff --git a/support/timespec.c b/support/timespec.c
new file mode 100644
index 0000000..c34fe73
--- /dev/null
+++ b/support/timespec.c
@@ -0,0 +1,63 @@
+/* Support code for timespec checks.
+   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/>.  */
+
+#include <support/timespec.h>
+#include <errno.h>
+#include <stdio.h>
+
+void
+test_timespec_before_impl (const char *file, int line,
+   const struct timespec left,
+   const struct timespec 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 ();
+    const struct timespec diff = timespec_sub (left, right);
+    printf ("%s:%d: %ld.%09lds not before %ld.%09lds "
+    "(difference %ld.%09lds)n",
+    file, line,
+    left.tv_sec, left.tv_nsec,
+    right.tv_sec, right.tv_nsec,
+    diff.tv_sec, diff.tv_nsec);
+    errno = saved_errno;
+  }
+}
+
+void
+test_timespec_equal_or_after_impl (const char *file, int line,
+   const struct timespec left,
+   const struct timespec 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 ();
+    const struct timespec diff = timespec_sub (right, left);
+    printf ("%s:%d: %ld.%09lds not after %ld.%09lds "
+    "(difference %ld.%09lds)n",
+    file, line,
+    left.tv_sec, left.tv_nsec,
+    right.tv_sec, right.tv_nsec,
+    diff.tv_sec, diff.tv_nsec);
+    errno = saved_errno;
+  }
+}
diff --git a/support/timespec.h b/support/timespec.h
new file mode 100644
index 0000000..4a8b341
--- /dev/null
+++ b/support/timespec.h
@@ -0,0 +1,79 @@
+/* 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>
+
+struct timespec timespec_add (struct timespec, struct timespec)
+  __attribute__((const));
+struct timespec timespec_sub (struct timespec, struct timespec)
+  __attribute__((const));
+
+static inline struct timespec
+make_timespec (time_t s, long int ns)
+{
+  struct timespec r;
+  r.tv_sec = s;
+  r.tv_nsec = ns;
+  return r;
+}
+
+enum { TIMESPEC_HZ = 1000000000 };
+
+void test_timespec_before_impl (const char *file, int line,
+                                const struct timespec left,
+                                const struct timespec right);
+
+void test_timespec_equal_or_after_impl (const char *file, int line,
+                                        const struct timespec left,
+                                        const struct timespec right);
+
+/* Check that the timespec on the left represents a time before the
+   time on the right. */
+#define TEST_TIMESPEC_BEFORE(left, right)                               \
+  test_timespec_before_impl (__FILE__, __LINE__, (left), (right))
+
+#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)                       \
+  test_timespec_equal_or_after_impl (__FILE__, __LINE__, left, right)
+
+#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
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 3/6] nptl: Convert tst-cond11.c to use libsupport

Mike Crowe
In reply to this post by Mike Crowe
* nptl/tst-cond11.c: Use libsupport.
---
 ChangeLog         |   4 +-
 nptl/tst-cond11.c | 169 ++++++++++-------------------------------------
 2 files changed, 40 insertions(+), 133 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 49dc84e..5aafd69 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2019-04-06  Mike Crowe  <[hidden email]>
 
+ * nptl/tst-cond11.c: Use libsupport.
+
+2019-04-06  Mike Crowe  <[hidden email]>
+
  * support/timespec.h: New file. Provide timespec helper functions
  along with macros in the style of those in check.h.
 
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>
--
git-series 0.9.1
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 4/6] nptl: Use recent additions to libsupport in tst-sem5

Mike Crowe
In reply to this post by Mike Crowe
* nptl/tst-sem5.c(do_test): Use xclock_gettime, timespec_add and
        TEST_TIMESPEC_NOW_OR_AFTER from libsupport.
---
 ChangeLog       |  5 +++++
 nptl/tst-sem5.c | 23 +++++------------------
 2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5aafd69..c78863e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2019-04-06  Mike Crowe  <[hidden email]>
 
+ * nptl/tst-sem5.c(do_test): Use xclock_gettime, timespec_add and
+ TEST_TIMESPEC_NOW_OR_AFTER from libsupport.
+
+2019-04-06  Mike Crowe  <[hidden email]>
+
  * nptl/tst-cond11.c: Use libsupport.
 
 2019-04-06  Mike Crowe  <[hidden email]>
diff --git a/nptl/tst-sem5.c b/nptl/tst-sem5.c
index 50ab6f9..7e722bd 100644
--- a/nptl/tst-sem5.c
+++ b/nptl/tst-sem5.c
@@ -22,6 +22,8 @@
 #include <unistd.h>
 #include <sys/time.h>
 #include <support/check.h>
+#include <support/timespec.h>
+#include <support/xtime.h>
 
 
 static int
@@ -29,31 +31,16 @@ do_test (void)
 {
   sem_t s;
   struct timespec ts;
-  struct timeval tv;
 
   TEST_COMPARE (sem_init (&s, 0, 1), 0);
   TEST_COMPARE (TEMP_FAILURE_RETRY (sem_wait (&s)), 0);
-  TEST_COMPARE (gettimeofday (&tv, NULL), 0);
-
-  TIMEVAL_TO_TIMESPEC (&tv, &ts);
-
-  /* We wait for half a second.  */
-  ts.tv_nsec += 500000000;
-  if (ts.tv_nsec >= 1000000000)
-    {
-      ++ts.tv_sec;
-      ts.tv_nsec -= 1000000000;
-    }
+  xclock_gettime (CLOCK_REALTIME, &ts);
+  ts = timespec_add (ts, make_timespec (0, TIMESPEC_HZ/2));
 
   errno = 0;
   TEST_COMPARE (TEMP_FAILURE_RETRY (sem_timedwait (&s, &ts)), -1);
   TEST_COMPARE (errno, ETIMEDOUT);
-
-  struct timespec ts2;
-  TEST_COMPARE (clock_gettime (CLOCK_REALTIME, &ts2), 0);
-
-  TEST_VERIFY (ts2.tv_sec > ts.tv_sec
-               || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec > ts.tv_nsec));
+  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts);
 
   return 0;
 }
--
git-series 0.9.1
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 5/6] nptl: Convert some rwlock tests to use libsupport

Mike Crowe
In reply to this post by Mike Crowe
* nptl/tst-rwlock6.c: Fix small bug in timeout-checking code that
        could erroneously pass if the function incorrectly took more than a
        second.

        * nptl/tst-rwlock6.c: Use clock_gettime(2) rather than
        gettimeofday(2) and then converting to timespec in preparation for
        testing pthread_rwlock_clockrdclock and pthread_rwlock_clockwrlock.

        * nptl/tst-rwlock7.c, nptl/tst-rwlock9.c: Likewise.

        * support/check.h: Introduce FAIL_THREAD_EXIT1 and FAIL_PRINT
        macros.

        * nptl/tst-rwlock6.c, nptl/tst-rwlock7.c, nptl/tst-rwlock9.c,
        nptl/tst-rwlock14.c: Use libsupport.
---
 ChangeLog           |  10 +++-
 nptl/tst-rwlock14.c | 122 +++++---------------------------------
 nptl/tst-rwlock6.c  | 146 ++++++++-------------------------------------
 nptl/tst-rwlock7.c  | 119 ++++++++-----------------------------
 nptl/tst-rwlock9.c  |  96 ++++++------------------------
 5 files changed, 107 insertions(+), 386 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c78863e..929018e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2019-04-06  Mike Crowe  <[hidden email]>
 
+ * nptl/tst-rwlock6.c: Use libsupport. This also happens to fix a
+ small bug where only tv.tv_usec was checked which could cause an
+ erroneous pass if pthread_rwlock_timedrdlock incorrectly took more
+ than a second.
+
+ * nptl/tst-rwlock7.c, nptl/tst-rwlock9.c, nptl/tst-rwlock14.c: Use
+ libsupport.
+
+2019-04-06  Mike Crowe  <[hidden email]>
+
  * nptl/tst-sem5.c(do_test): Use xclock_gettime, timespec_add and
  TEST_TIMESPEC_NOW_OR_AFTER from libsupport.
 
diff --git a/nptl/tst-rwlock14.c b/nptl/tst-rwlock14.c
index 6f57169..af176b6 100644
--- a/nptl/tst-rwlock14.c
+++ b/nptl/tst-rwlock14.c
@@ -21,6 +21,9 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <time.h>
+#include <support/check.h>
+#include <support/xthread.h>
+#include <support/xtime.h>
 
 
 static pthread_barrier_t b;
@@ -31,18 +34,14 @@ static void *
 tf (void *arg)
 {
   /* Lock the read-write lock.  */
-  if (pthread_rwlock_wrlock (&r) != 0)
-    {
-      puts ("tf: cannot lock rwlock");
-      exit (EXIT_FAILURE);
-    }
+  TEST_COMPARE (pthread_rwlock_wrlock (&r), 0);
 
   pthread_t mt = *(pthread_t *) arg;
 
-  pthread_barrier_wait (&b);
+  xpthread_barrier_wait (&b);
 
   /* This call will never return.  */
-  pthread_join (mt, NULL);
+  xpthread_join (mt);
 
   return NULL;
 }
@@ -51,118 +50,35 @@ tf (void *arg)
 static int
 do_test (void)
 {
-  int result = 0;
   struct timespec ts;
 
-  if (clock_gettime (CLOCK_REALTIME, &ts) != 0)
-    {
-      puts ("clock_gettime failed");
-      return 1;
-    }
-
-  if (pthread_barrier_init (&b, NULL, 2) != 0)
-    {
-      puts ("barrier_init failed");
-      return 1;
-    }
+  xclock_gettime (CLOCK_REALTIME, &ts);
+  xpthread_barrier_init (&b, NULL, 2);
 
   pthread_t me = pthread_self ();
-  pthread_t th;
-  if (pthread_create (&th, NULL, tf, &me) != 0)
-    {
-      puts ("create failed");
-      return 1;
-    }
+  xpthread_create (NULL, tf, &me);
 
   /* Wait until the rwlock is locked.  */
-  pthread_barrier_wait (&b);
+  xpthread_barrier_wait (&b);
 
   ts.tv_nsec = -1;
 
-  int e = pthread_rwlock_timedrdlock (&r, &ts);
-  if (e == 0)
-    {
-      puts ("first rwlock_timedrdlock did not fail");
-      result = 1;
-    }
-  else if (e != EINVAL)
-    {
-      puts ("first rwlock_timedrdlock did not return EINVAL");
-      result = 1;
-    }
-
-  e = pthread_rwlock_timedwrlock (&r, &ts);
-  if (e == 0)
-    {
-      puts ("first rwlock_timedwrlock did not fail");
-      result = 1;
-    }
-  else if (e != EINVAL)
-    {
-      puts ("first rwlock_timedwrlock did not return EINVAL");
-      result = 1;
-    }
+  TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EINVAL);
+  TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EINVAL);
 
   ts.tv_nsec = 1000000000;
 
-  e = pthread_rwlock_timedrdlock (&r, &ts);
-  if (e == 0)
-    {
-      puts ("second rwlock_timedrdlock did not fail");
-      result = 1;
-    }
-  else if (e != EINVAL)
-    {
-      puts ("second rwlock_timedrdlock did not return EINVAL");
-      result = 1;
-    }
-
-  e = pthread_rwlock_timedwrlock (&r, &ts);
-  if (e == 0)
-    {
-      puts ("second rwlock_timedwrlock did not fail");
-      result = 1;
-    }
-  else if (e != EINVAL)
-    {
-      puts ("second rwlock_timedwrlock did not return EINVAL");
-      result = 1;
-    }
+  TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EINVAL);
+  TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EINVAL);
 
   ts.tv_nsec = (__typeof (ts.tv_nsec)) 0x100001000LL;
   if ((__typeof (ts.tv_nsec)) 0x100001000LL != 0x100001000LL)
     ts.tv_nsec = 2000000000;
 
-  e = pthread_rwlock_timedrdlock (&r, &ts);
-  if (e == 0)
-    {
-      puts ("third rwlock_timedrdlock did not fail");
-      result = 1;
-    }
-  else if (e != EINVAL)
-    {
-      puts ("third rwlock_timedrdlock did not return EINVAL");
-      result = 1;
-    }
-
-  e = pthread_rwlock_timedwrlock (&r, &ts);
-  if (e == 0)
-    {
-      puts ("third rwlock_timedwrlock did not fail");
-      result = 1;
-    }
-  else if (e != EINVAL)
-    {
-      puts ("third rwlock_timedwrlock did not return EINVAL");
-      result = 1;
-    }
-
-  if (result == 0)
-    puts ("no bugs");
-
-  return result;
-}
+  TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EINVAL);
+  TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EINVAL);
 
+  return 0;
+}
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/nptl/tst-rwlock6.c b/nptl/tst-rwlock6.c
index 8d6c3dc..5e73f50 100644
--- a/nptl/tst-rwlock6.c
+++ b/nptl/tst-rwlock6.c
@@ -22,6 +22,10 @@
 #include <stdio.h>
 #include <string.h>
 #include <sys/time.h>
+#include <support/check.h>
+#include <support/timespec.h>
+#include <support/xthread.h>
+#include <support/xtime.h>
 
 
 static int kind[] =
@@ -38,64 +42,26 @@ tf (void *arg)
   pthread_rwlock_t *r = arg;
 
   /* Timeout: 0.3 secs.  */
-  struct timeval tv;
-  (void) gettimeofday (&tv, NULL);
+  struct timespec ts_start;
+  xclock_gettime (CLOCK_REALTIME, &ts_start);
 
-  struct timespec ts;
-  TIMEVAL_TO_TIMESPEC (&tv, &ts);
-  ts.tv_nsec += 300000000;
-  if (ts.tv_nsec >= 1000000000)
-    {
-      ts.tv_nsec -= 1000000000;
-      ++ts.tv_sec;
-    }
+  struct timespec ts_timeout = timespec_add (ts_start,
+                                             make_timespec (0, 300000000));
 
   puts ("child calling timedrdlock");
 
-  int err = pthread_rwlock_timedrdlock (r, &ts);
-  if (err == 0)
-    {
-      puts ("rwlock_timedrdlock returned");
-      pthread_exit ((void *) 1l);
-    }
-
-  if (err != ETIMEDOUT)
-    {
-      printf ("err = %s (%d), expected %s (%d)\n",
-      strerror (err), err, strerror (ETIMEDOUT), ETIMEDOUT);
-      pthread_exit ((void *) 1l);
-    }
+  TEST_COMPARE (pthread_rwlock_timedrdlock (r, &ts_timeout), ETIMEDOUT);
 
   puts ("1st child timedrdlock done");
 
-  struct timeval tv2;
-  (void) gettimeofday (&tv2, NULL);
-
-  timersub (&tv2, &tv, &tv);
+  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts_timeout);
 
-  if (tv.tv_usec < 200000)
-    {
-      puts ("timeout too short");
-      pthread_exit ((void *) 1l);
-    }
-
-  (void) gettimeofday (&tv, NULL);
-  TIMEVAL_TO_TIMESPEC (&tv, &ts);
-  ts.tv_sec += 10;
+  xclock_gettime (CLOCK_REALTIME, &ts_timeout);
+  ts_timeout.tv_sec += 10;
   /* Note that the following operation makes ts invalid.  */
-  ts.tv_nsec += 1000000000;
+  ts_timeout.tv_nsec += 1000000000;
 
-  err = pthread_rwlock_timedrdlock (r, &ts);
-  if (err == 0)
-    {
-      puts ("2nd timedrdlock succeeded");
-      pthread_exit ((void *) 1l);
-    }
-  if (err != EINVAL)
-    {
-      puts ("2nd timedrdlock did not return EINVAL");
-      pthread_exit ((void *) 1l);
-    }
+  TEST_COMPARE (pthread_rwlock_timedrdlock (r, &ts_timeout), EINVAL);
 
   puts ("2nd child timedrdlock done");
 
@@ -113,113 +79,59 @@ do_test (void)
       pthread_rwlockattr_t a;
 
       if (pthread_rwlockattr_init (&a) != 0)
- {
-  printf ("round %Zu: rwlockattr_t failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlockattr_t failed\n", cnt);
 
       if (pthread_rwlockattr_setkind_np (&a, kind[cnt]) != 0)
- {
-  printf ("round %Zu: rwlockattr_setkind failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlockattr_setkind failed\n", cnt);
 
       if (pthread_rwlock_init (&r, &a) != 0)
- {
-  printf ("round %Zu: rwlock_init failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlock_init failed\n", cnt);
 
       if (pthread_rwlockattr_destroy (&a) != 0)
- {
-  printf ("round %Zu: rwlockattr_destroy failed\n", cnt);
-  exit (1);
- }
-
-      struct timeval tv;
-      (void) gettimeofday (&tv, NULL);
+        FAIL_EXIT1 ("round %Zu: rwlockattr_destroy failed\n", cnt);
 
       struct timespec ts;
-      TIMEVAL_TO_TIMESPEC (&tv, &ts);
-
+      xclock_gettime (CLOCK_REALTIME, &ts);
       ++ts.tv_sec;
 
       /* Get a write lock.  */
       int e = pthread_rwlock_timedwrlock (&r, &ts);
       if (e != 0)
- {
-  printf ("round %Zu: rwlock_timedwrlock failed (%d)\n", cnt, e);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlock_timedwrlock failed (%d)\n", cnt, e);
 
       puts ("1st timedwrlock done");
 
-      (void) gettimeofday (&tv, NULL);
-      TIMEVAL_TO_TIMESPEC (&tv, &ts);
+      xclock_gettime (CLOCK_REALTIME, &ts);
       ++ts.tv_sec;
-      e = pthread_rwlock_timedrdlock (&r, &ts);
-      if (e == 0)
- {
-  puts ("timedrdlock succeeded");
-  exit (1);
- }
-      if (e != EDEADLK)
- {
-  puts ("timedrdlock did not return EDEADLK");
-  exit (1);
- }
+      TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EDEADLK);
 
       puts ("1st timedrdlock done");
 
-      (void) gettimeofday (&tv, NULL);
-      TIMEVAL_TO_TIMESPEC (&tv, &ts);
+      xclock_gettime (CLOCK_REALTIME, &ts);
       ++ts.tv_sec;
-      e = pthread_rwlock_timedwrlock (&r, &ts);
-      if (e == 0)
- {
-  puts ("2nd timedwrlock succeeded");
-  exit (1);
- }
-      if (e != EDEADLK)
- {
-  puts ("2nd timedwrlock did not return EDEADLK");
-  exit (1);
- }
+      TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EDEADLK);
 
       puts ("2nd timedwrlock done");
 
       pthread_t th;
       if (pthread_create (&th, NULL, tf, &r) != 0)
- {
-  printf ("round %Zu: create failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: create failed\n", cnt);
 
       puts ("started thread");
 
       void *status;
       if (pthread_join (th, &status) != 0)
- {
-  printf ("round %Zu: join failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: join failed\n", cnt);
       if (status != NULL)
- {
-  printf ("failure in round %Zu\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("failure in round %Zu\n", cnt);
 
       puts ("joined thread");
 
       if (pthread_rwlock_destroy (&r) != 0)
- {
-  printf ("round %Zu: rwlock_destroy failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlock_destroy failed\n", cnt);
     }
 
   return 0;
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/nptl/tst-rwlock7.c b/nptl/tst-rwlock7.c
index 4d6f561..df50f0a 100644
--- a/nptl/tst-rwlock7.c
+++ b/nptl/tst-rwlock7.c
@@ -22,6 +22,10 @@
 #include <stdio.h>
 #include <string.h>
 #include <sys/time.h>
+#include <support/check.h>
+#include <support/timespec.h>
+#include <support/xthread.h>
+#include <support/xtime.h>
 
 
 static int kind[] =
@@ -38,61 +42,24 @@ tf (void *arg)
   pthread_rwlock_t *r = arg;
 
   /* Timeout: 0.3 secs.  */
-  struct timeval tv;
-  (void) gettimeofday (&tv, NULL);
+  struct timespec ts_start;
+  xclock_gettime (CLOCK_REALTIME, &ts_start);
+  const struct timespec ts_timeout = timespec_add (ts_start,
+                                                   make_timespec (0, 300000000));
 
-  struct timespec ts;
-  TIMEVAL_TO_TIMESPEC (&tv, &ts);
-  ts.tv_nsec += 300000000;
-  if (ts.tv_nsec >= 1000000000)
-    {
-      ts.tv_nsec -= 1000000000;
-      ++ts.tv_sec;
-    }
-
-  int err = pthread_rwlock_timedwrlock (r, &ts);
-  if (err == 0)
-    {
-      puts ("rwlock_timedwrlock returned");
-      pthread_exit ((void *) 1l);
-    }
-
-  if (err != ETIMEDOUT)
-    {
-      printf ("err = %s (%d), expected %s (%d)\n",
-      strerror (err), err, strerror (ETIMEDOUT), ETIMEDOUT);
-      pthread_exit ((void *) 1l);
-    }
+  TEST_COMPARE (pthread_rwlock_timedwrlock (r, &ts_timeout), ETIMEDOUT);
   puts ("child: timedwrlock failed with ETIMEDOUT");
 
-  struct timeval tv2;
-  (void) gettimeofday (&tv2, NULL);
-
-  timersub (&tv2, &tv, &tv);
+  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts_timeout);
 
-  if (tv.tv_usec < 200000)
-    {
-      puts ("timeout too short");
-      pthread_exit ((void *) 1l);
-    }
-
-  (void) gettimeofday (&tv, NULL);
-  TIMEVAL_TO_TIMESPEC (&tv, &ts);
-  ts.tv_sec += 10;
+  struct timespec ts_invalid;
+  xclock_gettime (CLOCK_REALTIME, &ts_invalid);
+  ts_invalid.tv_sec += 10;
   /* Note that the following operation makes ts invalid.  */
-  ts.tv_nsec += 1000000000;
+  ts_invalid.tv_nsec += 1000000000;
+
+  TEST_COMPARE (pthread_rwlock_timedwrlock (r, &ts_invalid), EINVAL);
 
-  err = pthread_rwlock_timedwrlock (r, &ts);
-  if (err == 0)
-    {
-      puts ("2nd timedwrlock succeeded");
-      pthread_exit ((void *) 1l);
-    }
-  if (err != EINVAL)
-    {
-      puts ("2nd timedwrlock did not return EINVAL");
-      pthread_exit ((void *) 1l);
-    }
   puts ("child: timedwrlock failed with EINVAL");
 
   return NULL;
@@ -109,73 +76,43 @@ do_test (void)
       pthread_rwlockattr_t a;
 
       if (pthread_rwlockattr_init (&a) != 0)
- {
-  printf ("round %Zu: rwlockattr_t failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlockattr_t failed\n", cnt);
 
       if (pthread_rwlockattr_setkind_np (&a, kind[cnt]) != 0)
- {
-  printf ("round %Zu: rwlockattr_setkind failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlockattr_setkind failed\n", cnt);
 
       if (pthread_rwlock_init (&r, &a) != 0)
- {
-  printf ("round %Zu: rwlock_init failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlock_init failed\n", cnt);
 
       if (pthread_rwlockattr_destroy (&a) != 0)
- {
-  printf ("round %Zu: rwlockattr_destroy failed\n", cnt);
-  exit (1);
- }
-
-      struct timeval tv;
-      (void) gettimeofday (&tv, NULL);
+        FAIL_EXIT1 ("round %Zu: rwlockattr_destroy failed\n", cnt);
 
       struct timespec ts;
-      TIMEVAL_TO_TIMESPEC (&tv, &ts);
+      xclock_gettime (CLOCK_REALTIME, &ts);
 
       ++ts.tv_sec;
 
       /* Get a read lock.  */
       if (pthread_rwlock_timedrdlock (&r, &ts) != 0)
- {
-  printf ("round %Zu: rwlock_timedrdlock failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlock_timedrdlock failed\n", cnt);
+
       printf ("%zu: got timedrdlock\n", cnt);
 
       pthread_t th;
       if (pthread_create (&th, NULL, tf, &r) != 0)
- {
-  printf ("round %Zu: create failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: create failed\n", cnt);
 
       void *status;
       if (pthread_join (th, &status) != 0)
- {
-  printf ("round %Zu: join failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: join failed\n", cnt);
       if (status != NULL)
- {
-  printf ("failure in round %Zu\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("failure in round %Zu\n", cnt);
 
       if (pthread_rwlock_destroy (&r) != 0)
- {
-  printf ("round %Zu: rwlock_destroy failed\n", cnt);
-  exit (1);
- }
+        FAIL_EXIT1 ("round %Zu: rwlock_destroy failed\n", cnt);
     }
 
   return 0;
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/nptl/tst-rwlock9.c b/nptl/tst-rwlock9.c
index 34f2d04..9dc126b 100644
--- a/nptl/tst-rwlock9.c
+++ b/nptl/tst-rwlock9.c
@@ -24,6 +24,8 @@
 #include <time.h>
 #include <unistd.h>
 #include <sys/time.h>
+#include <support/check.h>
+#include <support/timespec.h>
 
 
 #define NWRITERS 15
@@ -31,8 +33,8 @@
 #define NREADERS 15
 #define READTRIES 15
 
-#define TIMEOUT 1000000
-#define DELAY   1000000
+static const struct timespec timeout = {0,1000000};
+static const struct timespec delay = {0, 1000000};
 
 #ifndef KIND
 # define KIND PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
@@ -45,36 +47,23 @@ static void *
 writer_thread (void *nr)
 {
   struct timespec ts;
-  struct timespec delay;
   int n;
 
-  delay.tv_sec = 0;
-  delay.tv_nsec = DELAY;
-
   for (n = 0; n < WRITETRIES; ++n)
     {
       int e;
       do
  {
-  struct timeval tv;
-  (void) gettimeofday (&tv, NULL);
-  TIMEVAL_TO_TIMESPEC (&tv, &ts);
+  xclock_gettime (CLOCK_REALTIME, &ts);
 
-  ts.tv_nsec += 2 * TIMEOUT;
-  if (ts.tv_nsec >= 1000000000)
-    {
-      ts.tv_nsec -= 1000000000;
-      ++ts.tv_sec;
-    }
+          ts = timespec_add (ts, timeout);
+          ts = timespec_add (ts, timeout);
 
   printf ("writer thread %ld tries again\n", (long int) nr);
 
   e = pthread_rwlock_timedwrlock (&lock, &ts);
   if (e != 0 && e != ETIMEDOUT)
-    {
-      puts ("timedwrlock failed");
-      exit (1);
-    }
+            FAIL_EXIT1 ("timedwrlock failed");
  }
       while (e == ETIMEDOUT);
 
@@ -83,10 +72,7 @@ writer_thread (void *nr)
       nanosleep (&delay, NULL);
 
       if (pthread_rwlock_unlock (&lock) != 0)
- {
-  puts ("unlock for writer failed");
-  exit (1);
- }
+        FAIL_EXIT1 ("unlock for writer failed");
 
       printf ("writer thread %ld released\n", (long int) nr);
     }
@@ -99,36 +85,22 @@ static void *
 reader_thread (void *nr)
 {
   struct timespec ts;
-  struct timespec delay;
   int n;
 
-  delay.tv_sec = 0;
-  delay.tv_nsec = DELAY;
-
   for (n = 0; n < READTRIES; ++n)
     {
       int e;
       do
  {
-  struct timeval tv;
-  (void) gettimeofday (&tv, NULL);
-  TIMEVAL_TO_TIMESPEC (&tv, &ts);
+  xclock_gettime (CLOCK_REALTIME, &ts);
 
-  ts.tv_nsec += TIMEOUT;
-  if (ts.tv_nsec >= 1000000000)
-    {
-      ts.tv_nsec -= 1000000000;
-      ++ts.tv_sec;
-    }
+          ts = timespec_add (ts, timeout);
 
   printf ("reader thread %ld tries again\n", (long int) nr);
 
   e = pthread_rwlock_timedrdlock (&lock, &ts);
   if (e != 0 && e != ETIMEDOUT)
-    {
-      puts ("timedrdlock failed");
-      exit (1);
-    }
+            FAIL_EXIT1 ("timedrdlock failed");
  }
       while (e == ETIMEDOUT);
 
@@ -137,10 +109,7 @@ reader_thread (void *nr)
       nanosleep (&delay, NULL);
 
       if (pthread_rwlock_unlock (&lock) != 0)
- {
-  puts ("unlock for reader failed");
-  exit (1);
- }
+        FAIL_EXIT1 ("unlock for reader failed");
 
       printf ("reader thread %ld released\n", (long int) nr);
     }
@@ -159,22 +128,13 @@ do_test (void)
   pthread_rwlockattr_t a;
 
   if (pthread_rwlockattr_init (&a) != 0)
-    {
-      puts ("rwlockattr_t failed");
-      exit (1);
-    }
+    FAIL_EXIT1 ("rwlockattr_t failed");
 
   if (pthread_rwlockattr_setkind_np (&a, KIND) != 0)
-    {
-      puts ("rwlockattr_setkind failed");
-      exit (1);
-    }
+    FAIL_EXIT1 ("rwlockattr_setkind failed");
 
   if (pthread_rwlock_init (&lock, &a) != 0)
-    {
-      puts ("rwlock_init failed");
-      exit (1);
-    }
+    FAIL_EXIT1 ("rwlock_init failed");
 
   /* Make standard error the same as standard output.  */
   dup2 (1, 2);
@@ -185,37 +145,23 @@ do_test (void)
   for (n = 0; n < NWRITERS; ++n)
     if (pthread_create (&thwr[n], NULL, writer_thread,
  (void *) (long int) n) != 0)
-      {
- puts ("writer create failed");
- exit (1);
-      }
+      FAIL_EXIT1 ("writer create failed");
 
   for (n = 0; n < NREADERS; ++n)
     if (pthread_create (&thrd[n], NULL, reader_thread,
  (void *) (long int) n) != 0)
-      {
- puts ("reader create failed");
- exit (1);
-      }
+      FAIL_EXIT1 ("reader create failed");
 
   /* Wait for all the threads.  */
   for (n = 0; n < NWRITERS; ++n)
     if (pthread_join (thwr[n], &res) != 0)
-      {
- puts ("writer join failed");
- exit (1);
-      }
+      FAIL_EXIT1 ("writer join failed");
   for (n = 0; n < NREADERS; ++n)
     if (pthread_join (thrd[n], &res) != 0)
-      {
- puts ("reader join failed");
- exit (1);
-      }
+      FAIL_EXIT1 ("reader join failed");
 
   return 0;
 }
 
-#undef TIMEOUT
 #define TIMEOUT 30
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
--
git-series 0.9.1
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 6/6] nptl/tst-abstime: Use libsupport

Mike Crowe
In reply to this post by Mike Crowe
* nptl/tst-abstime.c: Use libsupport.
---
 ChangeLog          |  4 +++-
 nptl/tst-abstime.c | 70 ++++++++++++-----------------------------------
 2 files changed, 22 insertions(+), 52 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 929018e..43d7b65 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2019-04-06  Mike Crowe  <[hidden email]>
 
+ * nptl/tst-abstime.c: Use libsupport.
+
+2019-04-06  Mike Crowe  <[hidden email]>
+
  * nptl/tst-rwlock6.c: Use libsupport. This also happens to fix a
  small bug where only tv.tv_usec was checked which could cause an
  erroneous pass if pthread_rwlock_timedrdlock incorrectly took more
diff --git a/nptl/tst-abstime.c b/nptl/tst-abstime.c
index 71610f8..026be7e 100644
--- a/nptl/tst-abstime.c
+++ b/nptl/tst-abstime.c
@@ -20,6 +20,8 @@
 #include <pthread.h>
 #include <semaphore.h>
 #include <stdio.h>
+#include <support/check.h>
+#include <support/xthread.h>
 
 static pthread_cond_t c = PTHREAD_COND_INITIALIZER;
 static pthread_mutex_t m1 = PTHREAD_MUTEX_INITIALIZER;
@@ -31,67 +33,31 @@ static sem_t sem;
 static void *
 th (void *arg)
 {
-  long int res = 0;
-  int r;
   struct timespec t = { -2, 0 };
 
-  r = pthread_mutex_timedlock (&m1, &t);
-  if (r != ETIMEDOUT)
-    {
-      puts ("pthread_mutex_timedlock did not return ETIMEDOUT");
-      res = 1;
-    }
-  r = pthread_rwlock_timedrdlock (&rw1, &t);
-  if (r != ETIMEDOUT)
-    {
-      puts ("pthread_rwlock_timedrdlock did not return ETIMEDOUT");
-      res = 1;
-    }
-  r = pthread_rwlock_timedwrlock (&rw2, &t);
-  if (r != ETIMEDOUT)
-    {
-      puts ("pthread_rwlock_timedwrlock did not return ETIMEDOUT");
-      res = 1;
-    }
-  return (void *) res;
+  TEST_COMPARE (pthread_mutex_timedlock (&m1, &t), ETIMEDOUT);
+  TEST_COMPARE (pthread_rwlock_timedrdlock (&rw1, &t), ETIMEDOUT);
+  TEST_COMPARE (pthread_rwlock_timedwrlock (&rw2, &t), ETIMEDOUT);
+  return NULL;
 }
 
 static int
 do_test (void)
 {
-  int res = 0;
-  int r;
   struct timespec t = { -2, 0 };
-  pthread_t pth;
 
   sem_init (&sem, 0, 0);
-  r = sem_timedwait (&sem, &t);
-  if (r != -1 || errno != ETIMEDOUT)
-    {
-      puts ("sem_timedwait did not fail with ETIMEDOUT");
-      res = 1;
-    }
-
-  pthread_mutex_lock (&m1);
-  pthread_rwlock_wrlock (&rw1);
-  pthread_rwlock_rdlock (&rw2);
-  pthread_mutex_lock (&m2);
-  if (pthread_create (&pth, 0, th, 0) != 0)
-    {
-      puts ("cannot create thread");
-      return 1;
-    }
-  r = pthread_cond_timedwait (&c, &m2, &t);
-  if (r != ETIMEDOUT)
-    {
-      puts ("pthread_cond_timedwait did not return ETIMEDOUT");
-      res = 1;
-    }
-  void *thres;
-  pthread_join (pth, &thres);
-  return res | (thres != NULL);
+  TEST_COMPARE (sem_timedwait (&sem, &t), -1);
+  TEST_COMPARE (errno, ETIMEDOUT);
+
+  xpthread_mutex_lock (&m1);
+  xpthread_rwlock_wrlock (&rw1);
+  xpthread_rwlock_rdlock (&rw2);
+  xpthread_mutex_lock (&m2);
+  pthread_t pth = xpthread_create (0, th, 0);
+  TEST_COMPARE (pthread_cond_timedwait (&c, &m2, &t), ETIMEDOUT);
+  pthread_join (pth, NULL);
+  return 0;
 }
 
-
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
--
git-series 0.9.1
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 0/6] Add new helper functions+macros to libsupport and use them in some nptl tests

Mike Crowe
In reply to this post by Mike Crowe
On Sunday 07 April 2019 at 20:33:49 +0100, Mike Crowe wrote:

> In preparation for adding support for the _clockwait family of
> functions, modify various tests to:
>
> 1. use struct timespec rather than struct timeval
>
> 2. use libsupport.
>
> This requires adding a new support/timespec.h and support/xtime.h
> containing helper functions and macros along with timespec_sub and
> timespec_add implementations from gnulib.
>
> Thanks to Adhemerval Zanella for providing lots of help and advice
> with these patches. They haven't been reviewed yet, so the flaws are
> all my own.
>
> Changes since v1[1]:
>
> * Many whitespace fixes.
>
> * The check macros in timespec.h now call out-of-line functions rather
>   than containing the code themselves.
>
> * Take better timespec_add and timespec_sub implementations from
>   gnulib.
>
> * Use xclock_gettime in more places.
>
> * Split addition of support/timespec.h into a separate patch.
>
> * Combine two separate rwlock patches into one to hopefully make
>   review easier.
>
> * Use even more x* wrapper functions.
>
> [1] https://lwn.net/ml/libc-alpha/cover.13c2d68f411f9010956e8e52edfbe168964e1e5c.1553797867.git-series.mac%40mcrowe.com/
>
> Mike Crowe (6):
>   support: Add xclock_gettime
>   support: Add timespec-related helper functions
>   nptl: Convert tst-cond11.c to use libsupport
>   nptl: Use recent additions to libsupport in tst-sem5
>   nptl: Convert some rwlock tests to use libsupport
>   nptl/tst-abstime: Use libsupport
>
>  ChangeLog                |  51 ++++++++++++-
>  nptl/tst-abstime.c       |  70 ++++-------------
>  nptl/tst-cond11.c        | 169 ++++++++--------------------------------
>  nptl/tst-rwlock14.c      | 122 ++++-------------------------
>  nptl/tst-rwlock6.c       | 146 ++++++-----------------------------
>  nptl/tst-rwlock7.c       | 119 ++++++----------------------
>  nptl/tst-rwlock9.c       |  96 +++++------------------
>  nptl/tst-sem5.c          |  23 +----
>  support/Makefile         |   4 +-
>  support/README           |   6 +-
>  support/timespec-add.c   |  75 ++++++++++++++++++-
>  support/timespec-sub.c   |  75 ++++++++++++++++++-
>  support/timespec.c       |  63 +++++++++++++++-
>  support/timespec.h       |  79 +++++++++++++++++++-
>  support/xclock_gettime.c |  29 +++++++-
>  support/xtime.h          |  33 ++++++++-
>  16 files changed, 571 insertions(+), 589 deletions(-)
>  create mode 100644 support/timespec-add.c
>  create mode 100644 support/timespec-sub.c
>  create mode 100644 support/timespec.c
>  create mode 100644 support/timespec.h
>  create mode 100644 support/xclock_gettime.c
>  create mode 100644 support/xtime.h
>
> base-commit: 94e358f6d490650c714edb1ffc3a52f56ffe086e
> --
> git-series 0.9.1
>

Ping!
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/6] support: Add xclock_gettime

Adhemerval Zanella-2
In reply to this post by Mike Crowe


On 07/04/2019 16:33, Mike Crowe wrote:
> * support/xclock_gettime.c (xclock_gettime): New file. Provide
> clock_gettime wrapper for use in tests that fails the test rather
> than returning failure.
>
> * support/xtime.h: New file to declare xclock_gettime.
>
> * support/Makefile: Add xclock_gettime.c.
>
> * support/README: Mention xtime.h.

LGTM with the change below.

> ---
>  ChangeLog                | 12 ++++++++++++
>  support/Makefile         |  1 +
>  support/README           |  1 +
>  support/xclock_gettime.c | 29 +++++++++++++++++++++++++++++
>  support/xtime.h          | 33 +++++++++++++++++++++++++++++++++
>  5 files changed, 76 insertions(+)
>  create mode 100644 support/xclock_gettime.c
>  create mode 100644 support/xtime.h
>
> diff --git a/ChangeLog b/ChangeLog
> index 5ad8875..8c26806 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,15 @@
> +2019-04-06  Mike Crowe  <[hidden email]>
> +
> + * support/xclock_gettime.c (xclock_gettime): New file. Provide
> + clock_gettime wrapper for use in tests that fails the test rather
> + than returning failure.
> +
> + * support/xtime.h: New file to declare xclock_gettime.
> +
> + * support/Makefile: Add xclock_gettime.c.
> +
> + * support/README: Mention xtime.h.
> +
>  2019-04-05  Anton Youdkevitch <[hidden email]>
>  
>   * sysdeps/aarch64/multiarch/memcpy_thunderx2.S: Cleanup branching
> diff --git a/support/Makefile b/support/Makefile
> index f173565..1d37f70 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -77,6 +77,7 @@ libsupport-routines = \
>    xbind \
>    xcalloc \
>    xchroot \
> +  xclock_gettime \
>    xclose \
>    xconnect \
>    xcopy_file_range \

Ok.

> diff --git a/support/README b/support/README
> index 476cfcd..d82f472 100644
> --- a/support/README
> +++ b/support/README
> @@ -10,6 +10,7 @@ error.  They are declared in these header files:
>  * support.h
>  * xsignal.h
>  * xthread.h
> +* xtime.h
>  
>  In general, new wrappers should be added to support.h if possible.
>  However, support.h must remain fully compatible with C90 and therefore

Ok.

> diff --git a/support/xclock_gettime.c b/support/xclock_gettime.c
> new file mode 100644
> index 0000000..91464fe
> --- /dev/null
> +++ b/support/xclock_gettime.c
> @@ -0,0 +1,29 @@
> +/* clock_gettime with error checking.
> +   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/>.  */
> +
> +#include <support/xtime.h>
> +#include <support/xthread.h>
> +
> +
> +void
> +xclock_gettime (clockid_t clockid,
> +                struct timespec *ts)
> +{
> +  xpthread_check_return
> +    ("clock_gettime", clock_gettime (clockid, ts));
> +}

xpthread_check_return uses the returned values as errno, while clock_gettime sets
errno appropriately.  Just use other functions that set errno:

  int ret = clock_gettime (clockid, ts);
  if (ret < 0)
    FAIL_EXIT1 ("clock_gettime (\"%d\", {\"%ld\", \"%ld\"}): %m",
                clockid, (long int) ts.tv_sec, ts.tv_nsec);
  return ret

> diff --git a/support/xtime.h b/support/xtime.h
> new file mode 100644
> index 0000000..68af1a5
> --- /dev/null
> +++ b/support/xtime.h
> @@ -0,0 +1,33 @@
> +/* Support functionality for using time.
> +   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_TIME_H
> +#define SUPPORT_TIME_H
> +
> +#include <time.h>
> +
> +__BEGIN_DECLS
> +
> +/* The following functions call the corresponding libc functions and
> +   terminate the process on error.  */
> +
> +void xclock_gettime (clockid_t clock, struct timespec *ts);
> +
> +__END_DECLS
> +
> +#endif /* SUPPORT_TIME_H */
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 2/6] support: Add timespec-related helper functions

Adhemerval Zanella-2
In reply to this post by Mike Crowe


On 07/04/2019 16:33, Mike Crowe wrote:

> * support/timespec.h: New file. Provide timespec helper functions
> along with macros in the style of those in check.h.
>
> * support/timespec.c: New file. Implement check functions declared
> in support/timespec.h.
>
> * support/timespec-add.c: New file from gnulib containing
> timespec_add implementation that handles overflow.
>
> * support/timespec-sub.c: New file from gnulib containing
> timespec_sub implementation that handles overflow.
>
> * support/README: Mention timespec.h.
> ---
>  ChangeLog              | 16 +++++++++-
>  support/Makefile       |  3 ++-
>  support/README         |  5 +++-
>  support/timespec-add.c | 75 +++++++++++++++++++++++++++++++++++++++++-
>  support/timespec-sub.c | 75 +++++++++++++++++++++++++++++++++++++++++-
>  support/timespec.c     | 63 ++++++++++++++++++++++++++++++++++-
>  support/timespec.h     | 79 +++++++++++++++++++++++++++++++++++++++++++-
>  7 files changed, 316 insertions(+)
>  create mode 100644 support/timespec-add.c
>  create mode 100644 support/timespec-sub.c
>  create mode 100644 support/timespec.c
>  create mode 100644 support/timespec.h
>
> diff --git a/ChangeLog b/ChangeLog
> index 8c26806..49dc84e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,21 @@
>  2019-04-06  Mike Crowe  <[hidden email]>
>  
> + * support/timespec.h: New file. Provide timespec helper functions
> + along with macros in the style of those in check.h.
> +
> + * support/timespec.c: New file. Implement check functions declared
> + in support/timespec.h.
> +
> + * support/timespec-add.c: New file from gnulib containing
> + timespec_add implementation that handles overflow.
> +
> + * support/timespec-sub.c: New file from gnulib containing
> + timespec_sub implementation that handles overflow.
> +
> + * support/README: Mention timespec.h.
> +

Ok.

> +2019-04-06  Mike Crowe  <[hidden email]>
> +
>   * support/xclock_gettime.c (xclock_gettime): New file. Provide
>   clock_gettime wrapper for use in tests that fails the test rather
>   than returning failure.
> diff --git a/support/Makefile b/support/Makefile
> index 1d37f70..94a1416 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -70,6 +70,9 @@ libsupport-routines = \
>    support_test_main \
>    support_test_verify_impl \
>    temp_file \
> +  timespec \
> +  timespec-add \
> +  timespec-sub \
>    write_message \
>    xaccept \
>    xaccept4 \

Ok.

> diff --git a/support/README b/support/README
> index d82f472..e33067e 100644
> --- a/support/README
> +++ b/support/README
> @@ -28,3 +28,8 @@ header files provide related declarations:
>  * check.h
>  * temp_file.h
>  * test-driver.h
> +
> +For tests that make use of struct timespec, the following header file
> +contains additional macros and helper functions:
> +
> +* timespec.h

Ok.

> diff --git a/support/timespec-add.c b/support/timespec-add.c
> new file mode 100644
> index 0000000..523eac8
> --- /dev/null
> +++ b/support/timespec-add.c
> @@ -0,0 +1,75 @@
> +/* Add two struct timespec values.
> +
> +   Copyright (C) 2011-2019 Free Software Foundation, Inc.
> +
> +   This file is part of the GNU C Library, although it originally came
> +   from gnulib.

Based from previous gnulib additions, I think we can use default Glibc
header and maybe added a comment below that it came from gnulib
originally.

> +
> +   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/>.  */
> +
> +/* Written by Paul Eggert.  */

We don't add authorship anymore on new files. I think a better approach
is to comment it came from gnulib to keep track for future syncs.

> +
> +/* Return the sum of two timespec values A and B.  On overflow, return
> +   an extremal value.  This assumes 0 <= tv_nsec < TIMESPEC_HZ.  */
> +
> +#include <config.h>
> +#include "timespec.h"
> +
> +#include "intprops.h"
> +
> +struct timespec
> +timespec_add (struct timespec a, struct timespec b)
> +{
> +  time_t rs = a.tv_sec;
> +  time_t bs = b.tv_sec;
> +  int ns = a.tv_nsec + b.tv_nsec;
> +  int nsd = ns - TIMESPEC_HZ;
> +  int rns = ns;
> +  time_t tmin = TYPE_MINIMUM (time_t);
> +  time_t tmax = TYPE_MAXIMUM (time_t);
> +
> +  if (0 <= nsd)
> +    {
> +      rns = nsd;
> +      if (bs < tmax)
> +        bs++;
> +      else if (rs < 0)
> +        rs++;
> +      else
> +        goto high_overflow;
> +    }
> +
> +  /* INT_ADD_WRAPV is not appropriate since time_t might be unsigned.
> +     In theory time_t might be narrower than int, so plain
> +     INT_ADD_OVERFLOW does not suffice.  */
> +  if (! INT_ADD_OVERFLOW (rs, bs) && tmin <= rs + bs && rs + bs <= tmax)
> +    rs += bs;
> +  else
> +    {
> +      if (rs < 0)
> +        {
> +          rs = tmin;
> +          rns = 0;
> +        }
> +      else
> +        {
> +        high_overflow:
> +          rs = tmax;
> +          rns = TIMESPEC_HZ - 1;
> +        }
> +    }
> +
> +  return make_timespec (rs, rns);
> +}

Ok.

> diff --git a/support/timespec-sub.c b/support/timespec-sub.c
> new file mode 100644
> index 0000000..7e67e87
> --- /dev/null
> +++ b/support/timespec-sub.c
> @@ -0,0 +1,75 @@
> +/* Subtract two struct timespec values.
> +
> +   Copyright (C) 2011-2019 Free Software Foundation, Inc.
> +
> +   This file is part of the GNU C Library, although it originally came
> +   from gnulib.

Same as before.

> +
> +   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/>.  */
> +
> +/* Written by Paul Eggert.  */

Ditto.

> +
> +/* Return the difference between two timespec values A and B.  On
> +   overflow, return an extremal value.  This assumes 0 <= tv_nsec <
> +   TIMESPEC_HZ.  */
> +
> +#include <config.h>
> +#include "timespec.h"
> +
> +#include "intprops.h"
> +
> +struct timespec
> +timespec_sub (struct timespec a, struct timespec b)
> +{
> +  time_t rs = a.tv_sec;
> +  time_t bs = b.tv_sec;
> +  int ns = a.tv_nsec - b.tv_nsec;
> +  int rns = ns;
> +  time_t tmin = TYPE_MINIMUM (time_t);
> +  time_t tmax = TYPE_MAXIMUM (time_t);
> +
> +  if (ns < 0)
> +    {
> +      rns = ns + TIMESPEC_HZ;
> +      if (bs < tmax)
> +        bs++;
> +      else if (- TYPE_SIGNED (time_t) < rs)
> +        rs--;
> +      else
> +        goto low_overflow;
> +    }
> +
> +  /* INT_SUBTRACT_WRAPV is not appropriate since time_t might be unsigned.
> +     In theory time_t might be narrower than int, so plain
> +     INT_SUBTRACT_OVERFLOW does not suffice.  */
> +  if (! INT_SUBTRACT_OVERFLOW (rs, bs) && tmin <= rs - bs && rs - bs <= tmax)
> +    rs -= bs;
> +  else
> +    {
> +      if (rs < 0)
> +        {
> +        low_overflow:
> +          rs = tmin;
> +          rns = 0;
> +        }
> +      else
> +        {
> +          rs = tmax;
> +          rns = TIMESPEC_HZ - 1;
> +        }
> +    }
> +
> +  return make_timespec (rs, rns);
> +}

Ok.

> diff --git a/support/timespec.c b/support/timespec.c
> new file mode 100644
> index 0000000..c34fe73
> --- /dev/null
> +++ b/support/timespec.c
> @@ -0,0 +1,63 @@
> +/* Support code for timespec checks.
> +   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/>.  */
> +
> +#include <support/timespec.h>
> +#include <errno.h>
> +#include <stdio.h>
> +
> +void
> +test_timespec_before_impl (const char *file, int line,
> +   const struct timespec left,
> +   const struct timespec 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 ();
> +    const struct timespec diff = timespec_sub (left, right);
> +    printf ("%s:%d: %ld.%09lds not before %ld.%09lds "
> +    "(difference %ld.%09lds)n",
> +    file, line,
> +    left.tv_sec, left.tv_nsec,
> +    right.tv_sec, right.tv_nsec,
> +    diff.tv_sec, diff.tv_nsec);
> +    errno = saved_errno;
> +  }
> +}

I think there is no need to save and restore errno, unless you are
actively testing errno after an expected failure (which is not the
case for the patchset and it not usual for libsupport and/or glibc
tests as well).

> +
> +void
> +test_timespec_equal_or_after_impl (const char *file, int line,
> +   const struct timespec left,
> +   const struct timespec 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 ();
> +    const struct timespec diff = timespec_sub (right, left);
> +    printf ("%s:%d: %ld.%09lds not after %ld.%09lds "
> +    "(difference %ld.%09lds)n",
> +    file, line,
> +    left.tv_sec, left.tv_nsec,
> +    right.tv_sec, right.tv_nsec,
> +    diff.tv_sec, diff.tv_nsec);
> +    errno = saved_errno;
> +  }
> +}

Ditto.

> diff --git a/support/timespec.h b/support/timespec.h
> new file mode 100644
> index 0000000..4a8b341
> --- /dev/null
> +++ b/support/timespec.h
> @@ -0,0 +1,79 @@
> +/* 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>
> +
> +struct timespec timespec_add (struct timespec, struct timespec)
> +  __attribute__((const));
> +struct timespec timespec_sub (struct timespec, struct timespec)
> +  __attribute__((const));
> +
> +static inline struct timespec
> +make_timespec (time_t s, long int ns)
> +{
> +  struct timespec r;
> +  r.tv_sec = s;
> +  r.tv_nsec = ns;
> +  return r;
> +}
> +
> +enum { TIMESPEC_HZ = 1000000000 };
> +
> +void test_timespec_before_impl (const char *file, int line,
> +                                const struct timespec left,
> +                                const struct timespec right);
> +
> +void test_timespec_equal_or_after_impl (const char *file, int line,
> +                                        const struct timespec left,
> +                                        const struct timespec right);
> +
> +/* Check that the timespec on the left represents a time before the
> +   time on the right. */
> +#define TEST_TIMESPEC_BEFORE(left, right)                               \
> +  test_timespec_before_impl (__FILE__, __LINE__, (left), (right))
> +
> +#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;                                        \
> +  })

Same as before for errno, I don't see much point on save/restore errno
for tests itself.

> +
> +/* Check that the timespec on the left represents a after before the
> +   time on the right. */
> +#define TEST_TIMESPEC_EQUAL_OR_AFTER(left, right)                       \
> +  test_timespec_equal_or_after_impl (__FILE__, __LINE__, left, right)
> +
> +#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 */
>

Same as before.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 3/6] nptl: Convert tst-cond11.c to use libsupport

Adhemerval Zanella-2
In reply to this post by Mike Crowe
LGTM.

On 07/04/2019 16:33, Mike Crowe wrote:

> * nptl/tst-cond11.c: Use libsupport.
> ---
>  ChangeLog         |   4 +-
>  nptl/tst-cond11.c | 169 ++++++++++-------------------------------------
>  2 files changed, 40 insertions(+), 133 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 49dc84e..5aafd69 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,9 @@
>  2019-04-06  Mike Crowe  <[hidden email]>
>  
> + * nptl/tst-cond11.c: Use libsupport.
> +
> +2019-04-06  Mike Crowe  <[hidden email]>
> +
>   * support/timespec.h: New file. Provide timespec helper functions
>   along with macros in the style of those in check.h.
>  

Ok.

> 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);

Ok.

>  
>    /* 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
>  }
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 4/6] nptl: Use recent additions to libsupport in tst-sem5

Adhemerval Zanella-2
In reply to this post by Mike Crowe
LGTM the above change.

On 07/04/2019 16:33, Mike Crowe wrote:
> * nptl/tst-sem5.c(do_test): Use xclock_gettime, timespec_add and
> TEST_TIMESPEC_NOW_OR_AFTER from libsupport.

Ok.

> ---
>  ChangeLog       |  5 +++++
>  nptl/tst-sem5.c | 23 +++++------------------
>  2 files changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 5aafd69..c78863e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,10 @@
>  2019-04-06  Mike Crowe  <[hidden email]>
>  
> + * nptl/tst-sem5.c(do_test): Use xclock_gettime, timespec_add and
> + TEST_TIMESPEC_NOW_OR_AFTER from libsupport.
> +
> +2019-04-06  Mike Crowe  <[hidden email]>
> +
>   * nptl/tst-cond11.c: Use libsupport.
>  
>  2019-04-06  Mike Crowe  <[hidden email]>
> diff --git a/nptl/tst-sem5.c b/nptl/tst-sem5.c
> index 50ab6f9..7e722bd 100644
> --- a/nptl/tst-sem5.c
> +++ b/nptl/tst-sem5.c
> @@ -22,6 +22,8 @@
>  #include <unistd.h>
>  #include <sys/time.h>
>  #include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xtime.h>
>  
>  
>  static int
> @@ -29,31 +31,16 @@ do_test (void)
>  {
>    sem_t s;
>    struct timespec ts;
> -  struct timeval tv;
>  
>    TEST_COMPARE (sem_init (&s, 0, 1), 0);
>    TEST_COMPARE (TEMP_FAILURE_RETRY (sem_wait (&s)), 0);
> -  TEST_COMPARE (gettimeofday (&tv, NULL), 0);
> -
> -  TIMEVAL_TO_TIMESPEC (&tv, &ts);
> -
> -  /* We wait for half a second.  */
> -  ts.tv_nsec += 500000000;
> -  if (ts.tv_nsec >= 1000000000)
> -    {
> -      ++ts.tv_sec;
> -      ts.tv_nsec -= 1000000000;
> -    }
> +  xclock_gettime (CLOCK_REALTIME, &ts);
> +  ts = timespec_add (ts, make_timespec (0, TIMESPEC_HZ/2));

Don't remove the comment.

>  
>    errno = 0;
>    TEST_COMPARE (TEMP_FAILURE_RETRY (sem_timedwait (&s, &ts)), -1);
>    TEST_COMPARE (errno, ETIMEDOUT);
> -
> -  struct timespec ts2;
> -  TEST_COMPARE (clock_gettime (CLOCK_REALTIME, &ts2), 0);
> -
> -  TEST_VERIFY (ts2.tv_sec > ts.tv_sec
> -               || (ts2.tv_sec == ts.tv_sec && ts2.tv_nsec > ts.tv_nsec));
> +  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts);
>  
>    return 0;
>  }
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 5/6] nptl: Convert some rwlock tests to use libsupport

Adhemerval Zanella-2
In reply to this post by Mike Crowe


On 07/04/2019 16:33, Mike Crowe wrote:

> * nptl/tst-rwlock6.c: Fix small bug in timeout-checking code that
> could erroneously pass if the function incorrectly took more than a
> second.
>
> * nptl/tst-rwlock6.c: Use clock_gettime(2) rather than
> gettimeofday(2) and then converting to timespec in preparation for
> testing pthread_rwlock_clockrdclock and pthread_rwlock_clockwrlock.
>
> * nptl/tst-rwlock7.c, nptl/tst-rwlock9.c: Likewise.
>
> * support/check.h: Introduce FAIL_THREAD_EXIT1 and FAIL_PRINT
> macros.
>
> * nptl/tst-rwlock6.c, nptl/tst-rwlock7.c, nptl/tst-rwlock9.c,
> nptl/tst-rwlock14.c: Use libsupport.

This CL entry does not seems to be in sync with patch. I think you meant
to use the one below.

Besides the out of the sync CL, LGTM with a small nit on tst-rwlock9.c.

> ---
>  ChangeLog           |  10 +++-
>  nptl/tst-rwlock14.c | 122 +++++---------------------------------
>  nptl/tst-rwlock6.c  | 146 ++++++++-------------------------------------
>  nptl/tst-rwlock7.c  | 119 ++++++++-----------------------------
>  nptl/tst-rwlock9.c  |  96 ++++++------------------------
>  5 files changed, 107 insertions(+), 386 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index c78863e..929018e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,15 @@
>  2019-04-06  Mike Crowe  <[hidden email]>
>  
> + * nptl/tst-rwlock6.c: Use libsupport. This also happens to fix a
> + small bug where only tv.tv_usec was checked which could cause an
> + erroneous pass if pthread_rwlock_timedrdlock incorrectly took more
> + than a second.
> +
> + * nptl/tst-rwlock7.c, nptl/tst-rwlock9.c, nptl/tst-rwlock14.c: Use
> + libsupport.
> +

Ok.

> +2019-04-06  Mike Crowe  <[hidden email]>
> +
>   * nptl/tst-sem5.c(do_test): Use xclock_gettime, timespec_add and
>   TEST_TIMESPEC_NOW_OR_AFTER from libsupport.
>  
> diff --git a/nptl/tst-rwlock14.c b/nptl/tst-rwlock14.c
> index 6f57169..af176b6 100644
> --- a/nptl/tst-rwlock14.c
> +++ b/nptl/tst-rwlock14.c
> @@ -21,6 +21,9 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <time.h>
> +#include <support/check.h>
> +#include <support/xthread.h>
> +#include <support/xtime.h>
>  
>  
>  static pthread_barrier_t b;
> @@ -31,18 +34,14 @@ static void *
>  tf (void *arg)
>  {
>    /* Lock the read-write lock.  */
> -  if (pthread_rwlock_wrlock (&r) != 0)
> -    {
> -      puts ("tf: cannot lock rwlock");
> -      exit (EXIT_FAILURE);
> -    }
> +  TEST_COMPARE (pthread_rwlock_wrlock (&r), 0);

Ok.

>  
>    pthread_t mt = *(pthread_t *) arg;
>  
> -  pthread_barrier_wait (&b);
> +  xpthread_barrier_wait (&b);
>  
>    /* This call will never return.  */
> -  pthread_join (mt, NULL);
> +  xpthread_join (mt);
>  
>    return NULL;
>  }

Ok.

> @@ -51,118 +50,35 @@ tf (void *arg)
>  static int
>  do_test (void)
>  {
> -  int result = 0;
>    struct timespec ts;
>  
> -  if (clock_gettime (CLOCK_REALTIME, &ts) != 0)
> -    {
> -      puts ("clock_gettime failed");
> -      return 1;
> -    }
> -
> -  if (pthread_barrier_init (&b, NULL, 2) != 0)
> -    {
> -      puts ("barrier_init failed");
> -      return 1;
> -    }
> +  xclock_gettime (CLOCK_REALTIME, &ts);
> +  xpthread_barrier_init (&b, NULL, 2);

Ok.

>  
>    pthread_t me = pthread_self ();
> -  pthread_t th;
> -  if (pthread_create (&th, NULL, tf, &me) != 0)
> -    {
> -      puts ("create failed");
> -      return 1;
> -    }
> +  xpthread_create (NULL, tf, &me);
>  
>    /* Wait until the rwlock is locked.  */
> -  pthread_barrier_wait (&b);
> +  xpthread_barrier_wait (&b);
>  
>    ts.tv_nsec = -1;
>  

Ok.

> -  int e = pthread_rwlock_timedrdlock (&r, &ts);
> -  if (e == 0)
> -    {
> -      puts ("first rwlock_timedrdlock did not fail");
> -      result = 1;
> -    }
> -  else if (e != EINVAL)
> -    {
> -      puts ("first rwlock_timedrdlock did not return EINVAL");
> -      result = 1;
> -    }
> -
> -  e = pthread_rwlock_timedwrlock (&r, &ts);
> -  if (e == 0)
> -    {
> -      puts ("first rwlock_timedwrlock did not fail");
> -      result = 1;
> -    }
> -  else if (e != EINVAL)
> -    {
> -      puts ("first rwlock_timedwrlock did not return EINVAL");
> -      result = 1;
> -    }
> +  TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EINVAL);
> +  TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EINVAL);

Ok.

>  
>    ts.tv_nsec = 1000000000;
>  
> -  e = pthread_rwlock_timedrdlock (&r, &ts);
> -  if (e == 0)
> -    {
> -      puts ("second rwlock_timedrdlock did not fail");
> -      result = 1;
> -    }
> -  else if (e != EINVAL)
> -    {
> -      puts ("second rwlock_timedrdlock did not return EINVAL");
> -      result = 1;
> -    }
> -
> -  e = pthread_rwlock_timedwrlock (&r, &ts);
> -  if (e == 0)
> -    {
> -      puts ("second rwlock_timedwrlock did not fail");
> -      result = 1;
> -    }
> -  else if (e != EINVAL)
> -    {
> -      puts ("second rwlock_timedwrlock did not return EINVAL");
> -      result = 1;
> -    }
> +  TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EINVAL);
> +  TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EINVAL);

Ok.

>  
>    ts.tv_nsec = (__typeof (ts.tv_nsec)) 0x100001000LL;
>    if ((__typeof (ts.tv_nsec)) 0x100001000LL != 0x100001000LL)
>      ts.tv_nsec = 2000000000;
>  
> -  e = pthread_rwlock_timedrdlock (&r, &ts);
> -  if (e == 0)
> -    {
> -      puts ("third rwlock_timedrdlock did not fail");
> -      result = 1;
> -    }
> -  else if (e != EINVAL)
> -    {
> -      puts ("third rwlock_timedrdlock did not return EINVAL");
> -      result = 1;
> -    }
> -
> -  e = pthread_rwlock_timedwrlock (&r, &ts);
> -  if (e == 0)
> -    {
> -      puts ("third rwlock_timedwrlock did not fail");
> -      result = 1;
> -    }
> -  else if (e != EINVAL)
> -    {
> -      puts ("third rwlock_timedwrlock did not return EINVAL");
> -      result = 1;
> -    }
> -
> -  if (result == 0)
> -    puts ("no bugs");
> -
> -  return result;
> -}
> +  TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EINVAL);
> +  TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EINVAL);

Ok.

>  
> +  return 0;
> +}
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>

Ok.

> diff --git a/nptl/tst-rwlock6.c b/nptl/tst-rwlock6.c
> index 8d6c3dc..5e73f50 100644
> --- a/nptl/tst-rwlock6.c
> +++ b/nptl/tst-rwlock6.c
> @@ -22,6 +22,10 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <sys/time.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xthread.h>
> +#include <support/xtime.h>
>  
>  
>  static int kind[] =
> @@ -38,64 +42,26 @@ tf (void *arg)
>    pthread_rwlock_t *r = arg;
>  
>    /* Timeout: 0.3 secs.  */
> -  struct timeval tv;
> -  (void) gettimeofday (&tv, NULL);
> +  struct timespec ts_start;
> +  xclock_gettime (CLOCK_REALTIME, &ts_start);
>  
> -  struct timespec ts;
> -  TIMEVAL_TO_TIMESPEC (&tv, &ts);
> -  ts.tv_nsec += 300000000;
> -  if (ts.tv_nsec >= 1000000000)
> -    {
> -      ts.tv_nsec -= 1000000000;
> -      ++ts.tv_sec;
> -    }
> +  struct timespec ts_timeout = timespec_add (ts_start,
> +                                             make_timespec (0, 300000000));

Ok.

>  
>    puts ("child calling timedrdlock");
>  
> -  int err = pthread_rwlock_timedrdlock (r, &ts);
> -  if (err == 0)
> -    {
> -      puts ("rwlock_timedrdlock returned");
> -      pthread_exit ((void *) 1l);
> -    }
> -
> -  if (err != ETIMEDOUT)
> -    {
> -      printf ("err = %s (%d), expected %s (%d)\n",
> -      strerror (err), err, strerror (ETIMEDOUT), ETIMEDOUT);
> -      pthread_exit ((void *) 1l);
> -    }
> +  TEST_COMPARE (pthread_rwlock_timedrdlock (r, &ts_timeout), ETIMEDOUT);

Ok.

>  
>    puts ("1st child timedrdlock done");
>  
> -  struct timeval tv2;
> -  (void) gettimeofday (&tv2, NULL);
> -
> -  timersub (&tv2, &tv, &tv);
> +  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts_timeout);
>  
> -  if (tv.tv_usec < 200000)
> -    {
> -      puts ("timeout too short");
> -      pthread_exit ((void *) 1l);
> -    }
> -
> -  (void) gettimeofday (&tv, NULL);
> -  TIMEVAL_TO_TIMESPEC (&tv, &ts);
> -  ts.tv_sec += 10;
> +  xclock_gettime (CLOCK_REALTIME, &ts_timeout);
> +  ts_timeout.tv_sec += 10;
>    /* Note that the following operation makes ts invalid.  */
> -  ts.tv_nsec += 1000000000;
> +  ts_timeout.tv_nsec += 1000000000;
>  
> -  err = pthread_rwlock_timedrdlock (r, &ts);
> -  if (err == 0)
> -    {
> -      puts ("2nd timedrdlock succeeded");
> -      pthread_exit ((void *) 1l);
> -    }
> -  if (err != EINVAL)
> -    {
> -      puts ("2nd timedrdlock did not return EINVAL");
> -      pthread_exit ((void *) 1l);
> -    }
> +  TEST_COMPARE (pthread_rwlock_timedrdlock (r, &ts_timeout), EINVAL);

Ok.

>  
>    puts ("2nd child timedrdlock done");
>  
> @@ -113,113 +79,59 @@ do_test (void)
>        pthread_rwlockattr_t a;
>  
>        if (pthread_rwlockattr_init (&a) != 0)
> - {
> -  printf ("round %Zu: rwlockattr_t failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_t failed\n", cnt);
>  
>        if (pthread_rwlockattr_setkind_np (&a, kind[cnt]) != 0)
> - {
> -  printf ("round %Zu: rwlockattr_setkind failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_setkind failed\n", cnt);
>  
>        if (pthread_rwlock_init (&r, &a) != 0)
> - {
> -  printf ("round %Zu: rwlock_init failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlock_init failed\n", cnt);
>  
>        if (pthread_rwlockattr_destroy (&a) != 0)
> - {
> -  printf ("round %Zu: rwlockattr_destroy failed\n", cnt);
> -  exit (1);
> - }
> -
> -      struct timeval tv;
> -      (void) gettimeofday (&tv, NULL);
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_destroy failed\n", cnt);

Ok.

>  
>        struct timespec ts;
> -      TIMEVAL_TO_TIMESPEC (&tv, &ts);
> -
> +      xclock_gettime (CLOCK_REALTIME, &ts);
>        ++ts.tv_sec;
>  

Ok.

>        /* Get a write lock.  */
>        int e = pthread_rwlock_timedwrlock (&r, &ts);
>        if (e != 0)
> - {
> -  printf ("round %Zu: rwlock_timedwrlock failed (%d)\n", cnt, e);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlock_timedwrlock failed (%d)\n", cnt, e);

Ok.

>  
>        puts ("1st timedwrlock done");
>  
> -      (void) gettimeofday (&tv, NULL);
> -      TIMEVAL_TO_TIMESPEC (&tv, &ts);
> +      xclock_gettime (CLOCK_REALTIME, &ts);
>        ++ts.tv_sec;

Ok.

> -      e = pthread_rwlock_timedrdlock (&r, &ts);
> -      if (e == 0)
> - {
> -  puts ("timedrdlock succeeded");
> -  exit (1);
> - }
> -      if (e != EDEADLK)
> - {
> -  puts ("timedrdlock did not return EDEADLK");
> -  exit (1);
> - }
> +      TEST_COMPARE (pthread_rwlock_timedrdlock (&r, &ts), EDEADLK);

Ok.

>  
>        puts ("1st timedrdlock done");
>  
> -      (void) gettimeofday (&tv, NULL);
> -      TIMEVAL_TO_TIMESPEC (&tv, &ts);
> +      xclock_gettime (CLOCK_REALTIME, &ts);
>        ++ts.tv_sec;

Ok.

> -      e = pthread_rwlock_timedwrlock (&r, &ts);
> -      if (e == 0)
> - {
> -  puts ("2nd timedwrlock succeeded");
> -  exit (1);
> - }
> -      if (e != EDEADLK)
> - {
> -  puts ("2nd timedwrlock did not return EDEADLK");
> -  exit (1);
> - }
> +      TEST_COMPARE (pthread_rwlock_timedwrlock (&r, &ts), EDEADLK);

Ok.

>  
>        puts ("2nd timedwrlock done");
>  
>        pthread_t th;
>        if (pthread_create (&th, NULL, tf, &r) != 0)
> - {
> -  printf ("round %Zu: create failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: create failed\n", cnt);

Ok.

>  
>        puts ("started thread");
>  
>        void *status;
>        if (pthread_join (th, &status) != 0)
> - {
> -  printf ("round %Zu: join failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: join failed\n", cnt);
>        if (status != NULL)
> - {
> -  printf ("failure in round %Zu\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("failure in round %Zu\n", cnt);
>  
>        puts ("joined thread");
>  
>        if (pthread_rwlock_destroy (&r) != 0)
> - {
> -  printf ("round %Zu: rwlock_destroy failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlock_destroy failed\n", cnt);
>      }
>  
>    return 0;
>  }
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>

Ok.

> diff --git a/nptl/tst-rwlock7.c b/nptl/tst-rwlock7.c
> index 4d6f561..df50f0a 100644
> --- a/nptl/tst-rwlock7.c
> +++ b/nptl/tst-rwlock7.c
> @@ -22,6 +22,10 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <sys/time.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xthread.h>
> +#include <support/xtime.h>
>  
>  
>  static int kind[] =
> @@ -38,61 +42,24 @@ tf (void *arg)
>    pthread_rwlock_t *r = arg;
>  
>    /* Timeout: 0.3 secs.  */
> -  struct timeval tv;
> -  (void) gettimeofday (&tv, NULL);
> +  struct timespec ts_start;
> +  xclock_gettime (CLOCK_REALTIME, &ts_start);
> +  const struct timespec ts_timeout = timespec_add (ts_start,
> +                                                   make_timespec (0, 300000000));
>  
> -  struct timespec ts;
> -  TIMEVAL_TO_TIMESPEC (&tv, &ts);
> -  ts.tv_nsec += 300000000;
> -  if (ts.tv_nsec >= 1000000000)
> -    {
> -      ts.tv_nsec -= 1000000000;
> -      ++ts.tv_sec;
> -    }
> -
> -  int err = pthread_rwlock_timedwrlock (r, &ts);
> -  if (err == 0)
> -    {
> -      puts ("rwlock_timedwrlock returned");
> -      pthread_exit ((void *) 1l);
> -    }
> -
> -  if (err != ETIMEDOUT)
> -    {
> -      printf ("err = %s (%d), expected %s (%d)\n",
> -      strerror (err), err, strerror (ETIMEDOUT), ETIMEDOUT);
> -      pthread_exit ((void *) 1l);
> -    }
> +  TEST_COMPARE (pthread_rwlock_timedwrlock (r, &ts_timeout), ETIMEDOUT);
>    puts ("child: timedwrlock failed with ETIMEDOUT");

Ok.

>  
> -  struct timeval tv2;
> -  (void) gettimeofday (&tv2, NULL);
> -
> -  timersub (&tv2, &tv, &tv);
> +  TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts_timeout);

Ok.

>  
> -  if (tv.tv_usec < 200000)
> -    {
> -      puts ("timeout too short");
> -      pthread_exit ((void *) 1l);
> -    }
> -
> -  (void) gettimeofday (&tv, NULL);
> -  TIMEVAL_TO_TIMESPEC (&tv, &ts);
> -  ts.tv_sec += 10;
> +  struct timespec ts_invalid;
> +  xclock_gettime (CLOCK_REALTIME, &ts_invalid);
> +  ts_invalid.tv_sec += 10;
>    /* Note that the following operation makes ts invalid.  */
> -  ts.tv_nsec += 1000000000;
> +  ts_invalid.tv_nsec += 1000000000;
> +
> +  TEST_COMPARE (pthread_rwlock_timedwrlock (r, &ts_invalid), EINVAL);

Ok.

>  
> -  err = pthread_rwlock_timedwrlock (r, &ts);
> -  if (err == 0)
> -    {
> -      puts ("2nd timedwrlock succeeded");
> -      pthread_exit ((void *) 1l);
> -    }
> -  if (err != EINVAL)
> -    {
> -      puts ("2nd timedwrlock did not return EINVAL");
> -      pthread_exit ((void *) 1l);
> -    }
>    puts ("child: timedwrlock failed with EINVAL");
>  
>    return NULL;

Ok.

> @@ -109,73 +76,43 @@ do_test (void)
>        pthread_rwlockattr_t a;
>  
>        if (pthread_rwlockattr_init (&a) != 0)
> - {
> -  printf ("round %Zu: rwlockattr_t failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_t failed\n", cnt);
>  
>        if (pthread_rwlockattr_setkind_np (&a, kind[cnt]) != 0)
> - {
> -  printf ("round %Zu: rwlockattr_setkind failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_setkind failed\n", cnt);
>  
>        if (pthread_rwlock_init (&r, &a) != 0)
> - {
> -  printf ("round %Zu: rwlock_init failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlock_init failed\n", cnt);
>  
>        if (pthread_rwlockattr_destroy (&a) != 0)
> - {
> -  printf ("round %Zu: rwlockattr_destroy failed\n", cnt);
> -  exit (1);
> - }
> -
> -      struct timeval tv;
> -      (void) gettimeofday (&tv, NULL);
> +        FAIL_EXIT1 ("round %Zu: rwlockattr_destroy failed\n", cnt);

Ok.

>  
>        struct timespec ts;
> -      TIMEVAL_TO_TIMESPEC (&tv, &ts);
> +      xclock_gettime (CLOCK_REALTIME, &ts);
>  
>        ++ts.tv_sec;
>  
>        /* Get a read lock.  */

Ok.

>        if (pthread_rwlock_timedrdlock (&r, &ts) != 0)
> - {
> -  printf ("round %Zu: rwlock_timedrdlock failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlock_timedrdlock failed\n", cnt);
> +
>        printf ("%zu: got timedrdlock\n", cnt);
>  
>        pthread_t th;
>        if (pthread_create (&th, NULL, tf, &r) != 0)
> - {
> -  printf ("round %Zu: create failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: create failed\n", cnt);
>  
>        void *status;
>        if (pthread_join (th, &status) != 0)
> - {
> -  printf ("round %Zu: join failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: join failed\n", cnt);
>        if (status != NULL)
> - {
> -  printf ("failure in round %Zu\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("failure in round %Zu\n", cnt);
>  
>        if (pthread_rwlock_destroy (&r) != 0)
> - {
> -  printf ("round %Zu: rwlock_destroy failed\n", cnt);
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("round %Zu: rwlock_destroy failed\n", cnt);
>      }
>  
>    return 0;
>  }
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>

Ok.

> diff --git a/nptl/tst-rwlock9.c b/nptl/tst-rwlock9.c
> index 34f2d04..9dc126b 100644
> --- a/nptl/tst-rwlock9.c
> +++ b/nptl/tst-rwlock9.c
> @@ -24,6 +24,8 @@
>  #include <time.h>
>  #include <unistd.h>
>  #include <sys/time.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
>  
>  
>  #define NWRITERS 15
> @@ -31,8 +33,8 @@
>  #define NREADERS 15
>  #define READTRIES 15
>  
> -#define TIMEOUT 1000000
> -#define DELAY   1000000
> +static const struct timespec timeout = {0,1000000};
> +static const struct timespec delay = {0, 1000000};

Space before and after bracket.

>  
>  #ifndef KIND
>  # define KIND PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
> @@ -45,36 +47,23 @@ static void *
>  writer_thread (void *nr)
>  {
>    struct timespec ts;
> -  struct timespec delay;
>    int n;
>  
> -  delay.tv_sec = 0;
> -  delay.tv_nsec = DELAY;
> -
>    for (n = 0; n < WRITETRIES; ++n)
>      {
>        int e;
>        do
>   {
> -  struct timeval tv;
> -  (void) gettimeofday (&tv, NULL);
> -  TIMEVAL_TO_TIMESPEC (&tv, &ts);
> +  xclock_gettime (CLOCK_REALTIME, &ts);

Ok.

>  
> -  ts.tv_nsec += 2 * TIMEOUT;
> -  if (ts.tv_nsec >= 1000000000)
> -    {
> -      ts.tv_nsec -= 1000000000;
> -      ++ts.tv_sec;
> -    }
> +          ts = timespec_add (ts, timeout);
> +          ts = timespec_add (ts, timeout);

Ok.

>  
>    printf ("writer thread %ld tries again\n", (long int) nr);
>  
>    e = pthread_rwlock_timedwrlock (&lock, &ts);
>    if (e != 0 && e != ETIMEDOUT)
> -    {
> -      puts ("timedwrlock failed");
> -      exit (1);
> -    }
> +            FAIL_EXIT1 ("timedwrlock failed");
>   }
>        while (e == ETIMEDOUT);
>  

Ok.

> @@ -83,10 +72,7 @@ writer_thread (void *nr)
>        nanosleep (&delay, NULL);
>  
>        if (pthread_rwlock_unlock (&lock) != 0)
> - {
> -  puts ("unlock for writer failed");
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("unlock for writer failed");
>  
>        printf ("writer thread %ld released\n", (long int) nr);
>      }

Ok.

> @@ -99,36 +85,22 @@ static void *
>  reader_thread (void *nr)
>  {
>    struct timespec ts;
> -  struct timespec delay;
>    int n;
>  
> -  delay.tv_sec = 0;
> -  delay.tv_nsec = DELAY;
> -
>    for (n = 0; n < READTRIES; ++n)
>      {
>        int e;
>        do
>   {
> -  struct timeval tv;
> -  (void) gettimeofday (&tv, NULL);
> -  TIMEVAL_TO_TIMESPEC (&tv, &ts);
> +  xclock_gettime (CLOCK_REALTIME, &ts);

Ok.

>  
> -  ts.tv_nsec += TIMEOUT;
> -  if (ts.tv_nsec >= 1000000000)
> -    {
> -      ts.tv_nsec -= 1000000000;
> -      ++ts.tv_sec;
> -    }
> +          ts = timespec_add (ts, timeout);

Ok.

>  
>    printf ("reader thread %ld tries again\n", (long int) nr);
>  
>    e = pthread_rwlock_timedrdlock (&lock, &ts);
>    if (e != 0 && e != ETIMEDOUT)
> -    {
> -      puts ("timedrdlock failed");
> -      exit (1);
> -    }
> +            FAIL_EXIT1 ("timedrdlock failed");
>   }
>        while (e == ETIMEDOUT);
>  

OK.

> @@ -137,10 +109,7 @@ reader_thread (void *nr)
>        nanosleep (&delay, NULL);
>  
>        if (pthread_rwlock_unlock (&lock) != 0)
> - {
> -  puts ("unlock for reader failed");
> -  exit (1);
> - }
> +        FAIL_EXIT1 ("unlock for reader failed");
>  
>        printf ("reader thread %ld released\n", (long int) nr);
>      }

Ok.

> @@ -159,22 +128,13 @@ do_test (void)
>    pthread_rwlockattr_t a;
>  
>    if (pthread_rwlockattr_init (&a) != 0)
> -    {
> -      puts ("rwlockattr_t failed");
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("rwlockattr_t failed");
>  
>    if (pthread_rwlockattr_setkind_np (&a, KIND) != 0)
> -    {
> -      puts ("rwlockattr_setkind failed");
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("rwlockattr_setkind failed");
>  
>    if (pthread_rwlock_init (&lock, &a) != 0)
> -    {
> -      puts ("rwlock_init failed");
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("rwlock_init failed");
>  

Ok.

>    /* Make standard error the same as standard output.  */
>    dup2 (1, 2);
> @@ -185,37 +145,23 @@ do_test (void)
>    for (n = 0; n < NWRITERS; ++n)
>      if (pthread_create (&thwr[n], NULL, writer_thread,
>   (void *) (long int) n) != 0)
> -      {
> - puts ("writer create failed");
> - exit (1);
> -      }
> +      FAIL_EXIT1 ("writer create failed");
>  
>    for (n = 0; n < NREADERS; ++n)
>      if (pthread_create (&thrd[n], NULL, reader_thread,
>   (void *) (long int) n) != 0)
> -      {
> - puts ("reader create failed");
> - exit (1);
> -      }
> +      FAIL_EXIT1 ("reader create failed");
>  
>    /* Wait for all the threads.  */
>    for (n = 0; n < NWRITERS; ++n)
>      if (pthread_join (thwr[n], &res) != 0)
> -      {
> - puts ("writer join failed");
> - exit (1);
> -      }
> +      FAIL_EXIT1 ("writer join failed");
>    for (n = 0; n < NREADERS; ++n)
>      if (pthread_join (thrd[n], &res) != 0)
> -      {
> - puts ("reader join failed");
> - exit (1);
> -      }
> +      FAIL_EXIT1 ("reader join failed");
>  
>    return 0;
>  }
>  
> -#undef TIMEOUT
>  #define TIMEOUT 30
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
>


Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 6/6] nptl/tst-abstime: Use libsupport

Adhemerval Zanella-2
In reply to this post by Mike Crowe
LGTM with a nit below.

On 07/04/2019 16:33, Mike Crowe wrote:

> * nptl/tst-abstime.c: Use libsupport.
> ---
>  ChangeLog          |  4 +++-
>  nptl/tst-abstime.c | 70 ++++++++++++-----------------------------------
>  2 files changed, 22 insertions(+), 52 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 929018e..43d7b65 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,9 @@
>  2019-04-06  Mike Crowe  <[hidden email]>
>  
> + * nptl/tst-abstime.c: Use libsupport.
> +

Ok.

> +2019-04-06  Mike Crowe  <[hidden email]>
> +
>   * nptl/tst-rwlock6.c: Use libsupport. This also happens to fix a
>   small bug where only tv.tv_usec was checked which could cause an
>   erroneous pass if pthread_rwlock_timedrdlock incorrectly took more
> diff --git a/nptl/tst-abstime.c b/nptl/tst-abstime.c
> index 71610f8..026be7e 100644
> --- a/nptl/tst-abstime.c
> +++ b/nptl/tst-abstime.c
> @@ -20,6 +20,8 @@
>  #include <pthread.h>
>  #include <semaphore.h>
>  #include <stdio.h>
> +#include <support/check.h>
> +#include <support/xthread.h>
>  
>  static pthread_cond_t c = PTHREAD_COND_INITIALIZER;
>  static pthread_mutex_t m1 = PTHREAD_MUTEX_INITIALIZER;
> @@ -31,67 +33,31 @@ static sem_t sem;
>  static void *
>  th (void *arg)
>  {
> -  long int res = 0;
> -  int r;
>    struct timespec t = { -2, 0 };
>  
> -  r = pthread_mutex_timedlock (&m1, &t);
> -  if (r != ETIMEDOUT)
> -    {
> -      puts ("pthread_mutex_timedlock did not return ETIMEDOUT");
> -      res = 1;
> -    }
> -  r = pthread_rwlock_timedrdlock (&rw1, &t);
> -  if (r != ETIMEDOUT)
> -    {
> -      puts ("pthread_rwlock_timedrdlock did not return ETIMEDOUT");
> -      res = 1;
> -    }
> -  r = pthread_rwlock_timedwrlock (&rw2, &t);
> -  if (r != ETIMEDOUT)
> -    {
> -      puts ("pthread_rwlock_timedwrlock did not return ETIMEDOUT");
> -      res = 1;
> -    }
> -  return (void *) res;
> +  TEST_COMPARE (pthread_mutex_timedlock (&m1, &t), ETIMEDOUT);
> +  TEST_COMPARE (pthread_rwlock_timedrdlock (&rw1, &t), ETIMEDOUT);
> +  TEST_COMPARE (pthread_rwlock_timedwrlock (&rw2, &t), ETIMEDOUT);
> +  return NULL;
>  }

Ok.

>  
>  static int
>  do_test (void)
>  {
> -  int res = 0;
> -  int r;
>    struct timespec t = { -2, 0 };
> -  pthread_t pth;
>  
>    sem_init (&sem, 0, 0);
> -  r = sem_timedwait (&sem, &t);
> -  if (r != -1 || errno != ETIMEDOUT)
> -    {
> -      puts ("sem_timedwait did not fail with ETIMEDOUT");
> -      res = 1;
> -    }
> -
> -  pthread_mutex_lock (&m1);
> -  pthread_rwlock_wrlock (&rw1);
> -  pthread_rwlock_rdlock (&rw2);
> -  pthread_mutex_lock (&m2);
> -  if (pthread_create (&pth, 0, th, 0) != 0)
> -    {
> -      puts ("cannot create thread");
> -      return 1;
> -    }
> -  r = pthread_cond_timedwait (&c, &m2, &t);
> -  if (r != ETIMEDOUT)
> -    {
> -      puts ("pthread_cond_timedwait did not return ETIMEDOUT");
> -      res = 1;
> -    }
> -  void *thres;
> -  pthread_join (pth, &thres);
> -  return res | (thres != NULL);
> +  TEST_COMPARE (sem_timedwait (&sem, &t), -1);
> +  TEST_COMPARE (errno, ETIMEDOUT);
> +

Ok.

> +  xpthread_mutex_lock (&m1);
> +  xpthread_rwlock_wrlock (&rw1);
> +  xpthread_rwlock_rdlock (&rw2);
> +  xpthread_mutex_lock (&m2);
> +  pthread_t pth = xpthread_create (0, th, 0);
> +  TEST_COMPARE (pthread_cond_timedwait (&c, &m2, &t), ETIMEDOUT);
> +  pthread_join (pth, NULL);

xpthread_join maybe?

> +  return 0;
>  }
>  
> -
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/6] support: Add xclock_gettime

Mike Crowe
In reply to this post by Adhemerval Zanella-2
On Tuesday 23 April 2019 at 10:59:00 -0300, Adhemerval Zanella wrote:

> > diff --git a/support/xclock_gettime.c b/support/xclock_gettime.c
> > new file mode 100644
> > index 0000000..91464fe
> > --- /dev/null
> > +++ b/support/xclock_gettime.c
> > @@ -0,0 +1,29 @@
> > +/* clock_gettime with error checking.
> > +   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/>.  */
> > +
> > +#include <support/xtime.h>
> > +#include <support/xthread.h>
> > +
> > +
> > +void
> > +xclock_gettime (clockid_t clockid,
> > +                struct timespec *ts)
> > +{
> > +  xpthread_check_return
> > +    ("clock_gettime", clock_gettime (clockid, ts));
> > +}
>
> xpthread_check_return uses the returned values as errno, while clock_gettime sets
> errno appropriately.  Just use other functions that set errno:
>
>   int ret = clock_gettime (clockid, ts);
>   if (ret < 0)
>     FAIL_EXIT1 ("clock_gettime (\"%d\", {\"%ld\", \"%ld\"}): %m",
> clockid, (long int) ts.tv_sec, ts.tv_nsec);
>   return ret

Is it really worth outputting ts.tv_sec and ts.tv_nsec? If the call has
failed then they will just contain whatever was in them before the call -
which probably means uninitialised. Even worse, if clock_gettime fails with
EFAULT then attempting to read from ts will fault too.

I think I'd prefer just:

 FAIL_EXIT1 ("clock_gettime (%d): %m", clockid);

Are you happy with that?

Mike.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/6] support: Add xclock_gettime

Adhemerval Zanella-2


On 25/04/2019 09:21, Mike Crowe wrote:

> On Tuesday 23 April 2019 at 10:59:00 -0300, Adhemerval Zanella wrote:
>>> diff --git a/support/xclock_gettime.c b/support/xclock_gettime.c
>>> new file mode 100644
>>> index 0000000..91464fe
>>> --- /dev/null
>>> +++ b/support/xclock_gettime.c
>>> @@ -0,0 +1,29 @@
>>> +/* clock_gettime with error checking.
>>> +   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/>.  */
>>> +
>>> +#include <support/xtime.h>
>>> +#include <support/xthread.h>
>>> +
>>> +
>>> +void
>>> +xclock_gettime (clockid_t clockid,
>>> +                struct timespec *ts)
>>> +{
>>> +  xpthread_check_return
>>> +    ("clock_gettime", clock_gettime (clockid, ts));
>>> +}
>>
>> xpthread_check_return uses the returned values as errno, while clock_gettime sets
>> errno appropriately.  Just use other functions that set errno:
>>
>>   int ret = clock_gettime (clockid, ts);
>>   if (ret < 0)
>>     FAIL_EXIT1 ("clock_gettime (\"%d\", {\"%ld\", \"%ld\"}): %m",
>> clockid, (long int) ts.tv_sec, ts.tv_nsec);
>>   return ret
>
> Is it really worth outputting ts.tv_sec and ts.tv_nsec? If the call has
> failed then they will just contain whatever was in them before the call -
> which probably means uninitialised. Even worse, if clock_gettime fails with
> EFAULT then attempting to read from ts will fault too.

Not really, it was just a suggestion in fact.

>
> I think I'd prefer just:
>
>  FAIL_EXIT1 ("clock_gettime (%d): %m", clockid);
>
> Are you happy with that?

LGTM, thanks.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 2/6] support: Add timespec-related helper functions

Mike Crowe
In reply to this post by Adhemerval Zanella-2
On Tuesday 23 April 2019 at 14:12:56 -0300, Adhemerval Zanella wrote:

> > diff --git a/support/timespec-add.c b/support/timespec-add.c
> > new file mode 100644
> > index 0000000..523eac8
> > --- /dev/null
> > +++ b/support/timespec-add.c
> > @@ -0,0 +1,75 @@
> > +/* Add two struct timespec values.
> > +
> > +   Copyright (C) 2011-2019 Free Software Foundation, Inc.
> > +
> > +   This file is part of the GNU C Library, although it originally came
> > +   from gnulib.
>
> Based from previous gnulib additions, I think we can use default Glibc
> header and maybe added a comment below that it came from gnulib
> originally.

posix/getopt.c and other files contain:

 This file is part of the GNU C Library and is also part of gnulib.
 Patches to this file should be submitted to both projects.

followed by the standard GNU C copyright notice. Would that be acceptable?

> > +/* Written by Paul Eggert.  */
>
> We don't add authorship anymore on new files. I think a better approach
> is to comment it came from gnulib to keep track for future syncs.

OK.

> > diff --git a/support/timespec.c b/support/timespec.c
> > new file mode 100644
> > index 0000000..c34fe73
> > --- /dev/null
> > +++ b/support/timespec.c
[snip]

> > +void
> > +test_timespec_before_impl (const char *file, int line,
> > +   const struct timespec left,
> > +   const struct timespec 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 ();
> > +    const struct timespec diff = timespec_sub (left, right);
> > +    printf ("%s:%d: %ld.%09lds not before %ld.%09lds "
> > +    "(difference %ld.%09lds)n",
> > +    file, line,
> > +    left.tv_sec, left.tv_nsec,
> > +    right.tv_sec, right.tv_nsec,
> > +    diff.tv_sec, diff.tv_nsec);
> > +    errno = saved_errno;
> > +  }
> > +}
>
> I think there is no need to save and restore errno, unless you are
> actively testing errno after an expected failure (which is not the
> case for the patchset and it not usual for libsupport and/or glibc
> tests as well).

I can remove that. I only did so because support_test_compare_failure did
so.

Mike.