Re: test-lock compilation failure on mingw

[Bruno Haible]

Paul Eggert wrote:
> POSIX doesn't guarantee that pthread_t is a pointer type, so
> test-lock.c shouldn't be trying to print gl_thread_self ()
> with %p; that's not portable.  Perhaps all instances of
> code like this:
> 
>       dbgprintf ("Mutator %p before lock\n", gl_thread_self ());
> 
> should be rewritten like this:
> 
>       dbgprintf ("Mutator %p before lock\n", gl_thread_self_pointer ());
> 
> where gl_thread_self_pointer is a new primitive that is guaranteed to
> yield a pointer value?  The value could be null on platforms where
> pthread_t doesn't fit into void *.

Yes, I agree with all that. Implemented, tested, and pushed:


2011-06-09  Bruno Haible  <address@hidden>

        thread: Support pthreads-win32.
        * lib/glthread/thread.h (gl_thread_self): Define differently on
        pthreads-win32.
        (gl_null_thread): New declaration.
        (gl_thread_self_pointer): New macro.
        * lib/glthread/thread.c (gl_null_thread): New constant.
        * tests/test-lock.c: Use gl_thread_self_pointer instead of
        gl_thread_self.
        * tests/test-tls.c: Likewise.
        Suggested by Paul Eggert. Reported by Eric Blake.

--- lib/glthread/thread.c.orig  Thu Jun  9 12:36:29 2011
+++ lib/glthread/thread.c       Thu Jun  9 12:17:25 2011
@@ -29,6 +29,20 @@
 
 /* ========================================================================= */
 
+#if USE_POSIX_THREADS
+
+#include <pthread.h>
+
+#ifdef PTW32_VERSION
+
+const gl_thread_t gl_null_thread /* = { .p = NULL } */;
+
+#endif
+
+#endif
+
+/* ========================================================================= */
+
 #if USE_WIN32_THREADS
 
 #include <process.h>
--- lib/glthread/thread.h.orig  Thu Jun  9 12:36:29 2011
+++ lib/glthread/thread.h       Thu Jun  9 12:15:37 2011
@@ -47,6 +47,10 @@
        current = gl_thread_self ();
        extern gl_thread_t gl_thread_self (void);
 
+   Getting a reference to the current thread as a pointer, for debugging:
+       ptr = gl_thread_self_pointer ();
+       extern void * gl_thread_self_pointer (void);
+
    Terminating the current thread:
        gl_thread_exit (return_value);
        extern void gl_thread_exit (void *return_value) __attribute__ 
((noreturn));
@@ -147,8 +151,20 @@
     (pthread_in_use () ? pthread_sigmask (HOW, SET, OSET) : 0)
 # define glthread_join(THREAD, RETVALP) \
     (pthread_in_use () ? pthread_join (THREAD, RETVALP) : 0)
-# define gl_thread_self() \
-    (pthread_in_use () ? (void *) pthread_self () : NULL)
+# ifdef PTW32_VERSION
+   /* In pthreads-win32, pthread_t is a struct with a pointer field 'p' and
+      other fields.  */
+#  define gl_thread_self() \
+     (pthread_in_use () ? pthread_self () : gl_null_thread)
+#  define gl_thread_self_pointer() \
+     (pthread_in_use () ? pthread_self ().p : NULL)
+extern const gl_thread_t gl_null_thread;
+# else
+#  define gl_thread_self() \
+     (pthread_in_use () ? (void *) pthread_self () : NULL)
+#  define gl_thread_self_pointer() \
+     gl_thread_self ()
+# endif
 # define gl_thread_exit(RETVAL) \
     (pthread_in_use () ? pthread_exit (RETVAL) : 0)
 
@@ -205,7 +221,9 @@
 # define glthread_join(THREAD, RETVALP) \
     (pth_in_use () && !pth_join (THREAD, RETVALP) ? errno : 0)
 # define gl_thread_self() \
-    (pth_in_use () ? (void *) pth_self () : 0)
+    (pth_in_use () ? (void *) pth_self () : NULL)
+# define gl_thread_self_pointer() \
+    gl_thread_self ()
 # define gl_thread_exit(RETVAL) \
     (pth_in_use () ? pth_exit (RETVAL) : 0)
 # define glthread_atfork(PREPARE_FUNC, PARENT_FUNC, CHILD_FUNC) 0
@@ -257,7 +275,9 @@
 # define glthread_join(THREAD, RETVALP) \
     (thread_in_use () ? thr_join (THREAD, NULL, RETVALP) : 0)
 # define gl_thread_self() \
-    (thread_in_use () ? (void *) thr_self () : 0)
+    (thread_in_use () ? (void *) thr_self () : NULL)
+# define gl_thread_self_pointer() \
+    gl_thread_self ()
 # define gl_thread_exit(RETVAL) \
     (thread_in_use () ? thr_exit (RETVAL) : 0)
 # define glthread_atfork(PREPARE_FUNC, PARENT_FUNC, CHILD_FUNC) 0
@@ -298,6 +318,8 @@
     glthread_join_func (THREAD, RETVALP)
 # define gl_thread_self() \
     gl_thread_self_func ()
+# define gl_thread_self_pointer() \
+    gl_thread_self ()
 # define gl_thread_exit(RETVAL) \
     gl_thread_exit_func (RETVAL)
 # define glthread_atfork(PREPARE_FUNC, PARENT_FUNC, CHILD_FUNC) 0
@@ -323,6 +345,8 @@
 # define glthread_sigmask(HOW, SET, OSET) 0
 # define glthread_join(THREAD, RETVALP) 0
 # define gl_thread_self() NULL
+# define gl_thread_self_pointer() \
+    gl_thread_self ()
 # define gl_thread_exit(RETVAL) 0
 # define glthread_atfork(PREPARE_FUNC, PARENT_FUNC, CHILD_FUNC) 0
 
--- tests/test-lock.c.orig      Thu Jun  9 12:36:29 2011
+++ tests/test-lock.c   Thu Jun  9 12:08:25 2011
@@ -143,9 +143,9 @@
     {
       int i1, i2, value;
 
-      dbgprintf ("Mutator %p before lock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p before lock\n", gl_thread_self_pointer ());
       gl_lock_lock (my_lock);
-      dbgprintf ("Mutator %p after  lock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p after  lock\n", gl_thread_self_pointer ());
 
       i1 = random_account ();
       i2 = random_account ();
@@ -153,20 +153,20 @@
       account[i1] += value;
       account[i2] -= value;
 
-      dbgprintf ("Mutator %p before unlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p before unlock\n", gl_thread_self_pointer ());
       gl_lock_unlock (my_lock);
-      dbgprintf ("Mutator %p after  unlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p after  unlock\n", gl_thread_self_pointer ());
 
-      dbgprintf ("Mutator %p before check lock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p before check lock\n", gl_thread_self_pointer ());
       gl_lock_lock (my_lock);
       check_accounts ();
       gl_lock_unlock (my_lock);
-      dbgprintf ("Mutator %p after  check unlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p after  check unlock\n", gl_thread_self_pointer 
());
 
       yield ();
     }
 
-  dbgprintf ("Mutator %p dying.\n", gl_thread_self ());
+  dbgprintf ("Mutator %p dying.\n", gl_thread_self_pointer ());
   return NULL;
 }
 
@@ -177,16 +177,16 @@
 {
   while (!lock_checker_done)
     {
-      dbgprintf ("Checker %p before check lock\n", gl_thread_self ());
+      dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
       gl_lock_lock (my_lock);
       check_accounts ();
       gl_lock_unlock (my_lock);
-      dbgprintf ("Checker %p after  check unlock\n", gl_thread_self ());
+      dbgprintf ("Checker %p after  check unlock\n", gl_thread_self_pointer 
());
 
       yield ();
     }
 
-  dbgprintf ("Checker %p dying.\n", gl_thread_self ());
+  dbgprintf ("Checker %p dying.\n", gl_thread_self_pointer ());
   return NULL;
 }
 
@@ -233,9 +233,9 @@
     {
       int i1, i2, value;
 
-      dbgprintf ("Mutator %p before wrlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p before wrlock\n", gl_thread_self_pointer ());
       gl_rwlock_wrlock (my_rwlock);
-      dbgprintf ("Mutator %p after  wrlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p after  wrlock\n", gl_thread_self_pointer ());
 
       i1 = random_account ();
       i2 = random_account ();
@@ -243,14 +243,14 @@
       account[i1] += value;
       account[i2] -= value;
 
-      dbgprintf ("Mutator %p before unlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p before unlock\n", gl_thread_self_pointer ());
       gl_rwlock_unlock (my_rwlock);
-      dbgprintf ("Mutator %p after  unlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p after  unlock\n", gl_thread_self_pointer ());
 
       yield ();
     }
 
-  dbgprintf ("Mutator %p dying.\n", gl_thread_self ());
+  dbgprintf ("Mutator %p dying.\n", gl_thread_self_pointer ());
   return NULL;
 }
 
@@ -261,16 +261,16 @@
 {
   while (!rwlock_checker_done)
     {
-      dbgprintf ("Checker %p before check rdlock\n", gl_thread_self ());
+      dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer 
());
       gl_rwlock_rdlock (my_rwlock);
       check_accounts ();
       gl_rwlock_unlock (my_rwlock);
-      dbgprintf ("Checker %p after  check unlock\n", gl_thread_self ());
+      dbgprintf ("Checker %p after  check unlock\n", gl_thread_self_pointer 
());
 
       yield ();
     }
 
-  dbgprintf ("Checker %p dying.\n", gl_thread_self ());
+  dbgprintf ("Checker %p dying.\n", gl_thread_self_pointer ());
   return NULL;
 }
 
@@ -315,9 +315,9 @@
 {
   int i1, i2, value;
 
-  dbgprintf ("Mutator %p before lock\n", gl_thread_self ());
+  dbgprintf ("Mutator %p before lock\n", gl_thread_self_pointer ());
   gl_recursive_lock_lock (my_reclock);
-  dbgprintf ("Mutator %p after  lock\n", gl_thread_self ());
+  dbgprintf ("Mutator %p after  lock\n", gl_thread_self_pointer ());
 
   i1 = random_account ();
   i2 = random_account ();
@@ -329,9 +329,9 @@
   if (((unsigned int) rand () >> 3) % 2)
     recshuffle ();
 
-  dbgprintf ("Mutator %p before unlock\n", gl_thread_self ());
+  dbgprintf ("Mutator %p before unlock\n", gl_thread_self_pointer ());
   gl_recursive_lock_unlock (my_reclock);
-  dbgprintf ("Mutator %p after  unlock\n", gl_thread_self ());
+  dbgprintf ("Mutator %p after  unlock\n", gl_thread_self_pointer ());
 }
 
 static void *
@@ -343,16 +343,16 @@
     {
       recshuffle ();
 
-      dbgprintf ("Mutator %p before check lock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p before check lock\n", gl_thread_self_pointer ());
       gl_recursive_lock_lock (my_reclock);
       check_accounts ();
       gl_recursive_lock_unlock (my_reclock);
-      dbgprintf ("Mutator %p after  check unlock\n", gl_thread_self ());
+      dbgprintf ("Mutator %p after  check unlock\n", gl_thread_self_pointer 
());
 
       yield ();
     }
 
-  dbgprintf ("Mutator %p dying.\n", gl_thread_self ());
+  dbgprintf ("Mutator %p dying.\n", gl_thread_self_pointer ());
   return NULL;
 }
 
@@ -363,16 +363,16 @@
 {
   while (!reclock_checker_done)
     {
-      dbgprintf ("Checker %p before check lock\n", gl_thread_self ());
+      dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
       gl_recursive_lock_lock (my_reclock);
       check_accounts ();
       gl_recursive_lock_unlock (my_reclock);
-      dbgprintf ("Checker %p after  check unlock\n", gl_thread_self ());
+      dbgprintf ("Checker %p after  check unlock\n", gl_thread_self_pointer 
());
 
       yield ();
     }
 
-  dbgprintf ("Checker %p dying.\n", gl_thread_self ());
+  dbgprintf ("Checker %p dying.\n", gl_thread_self_pointer ());
   return NULL;
 }
 
@@ -444,7 +444,7 @@
         break;
 
       dbgprintf ("Contender %p waiting for signal for round %d\n",
-                 gl_thread_self (), repeat);
+                 gl_thread_self_pointer (), repeat);
 #if ENABLE_LOCKING
       /* Wait for the signal to go.  */
       gl_rwlock_rdlock (fire_signal[repeat]);
@@ -456,7 +456,7 @@
         yield ();
 #endif
       dbgprintf ("Contender %p got the     signal for round %d\n",
-                 gl_thread_self (), repeat);
+                 gl_thread_self_pointer (), repeat);
 
       /* Contend for execution.  */
       gl_once (once_control, once_execute);
--- tests/test-tls.c.orig       Thu Jun  9 12:36:29 2011
+++ tests/test-tls.c    Thu Jun  9 12:09:00 2011
@@ -89,7 +89,7 @@
   int i, j, repeat;
   unsigned int values[KEYS_COUNT];
 
-  dbgprintf ("Worker %p started\n", gl_thread_self ());
+  dbgprintf ("Worker %p started\n", gl_thread_self_pointer ());
 
   /* Initialize the per-thread storage.  */
   for (i = 0; i < KEYS_COUNT; i++)
@@ -102,28 +102,28 @@
   perhaps_yield ();
 
   /* Verify that the initial value is NULL.  */
-  dbgprintf ("Worker %p before initial verify\n", gl_thread_self ());
+  dbgprintf ("Worker %p before initial verify\n", gl_thread_self_pointer ());
   for (i = 0; i < KEYS_COUNT; i++)
     if (gl_tls_get (mykeys[i]) != NULL)
       abort ();
-  dbgprintf ("Worker %p after  initial verify\n", gl_thread_self ());
+  dbgprintf ("Worker %p after  initial verify\n", gl_thread_self_pointer ());
   perhaps_yield ();
 
   /* Initialize the per-thread storage.  */
-  dbgprintf ("Worker %p before first tls_set\n", gl_thread_self ());
+  dbgprintf ("Worker %p before first tls_set\n", gl_thread_self_pointer ());
   for (i = 0; i < KEYS_COUNT; i++)
     {
       unsigned int *ptr = (unsigned int *) malloc (sizeof (unsigned int));
       *ptr = values[i];
       gl_tls_set (mykeys[i], ptr);
     }
-  dbgprintf ("Worker %p after  first tls_set\n", gl_thread_self ());
+  dbgprintf ("Worker %p after  first tls_set\n", gl_thread_self_pointer ());
   perhaps_yield ();
 
   /* Shuffle around the pointers.  */
   for (repeat = REPEAT_COUNT; repeat > 0; repeat--)
     {
-      dbgprintf ("Worker %p doing value swapping\n", gl_thread_self ());
+      dbgprintf ("Worker %p doing value swapping\n", gl_thread_self_pointer 
());
       i = ((unsigned int) rand () >> 3) % KEYS_COUNT;
       j = ((unsigned int) rand () >> 3) % KEYS_COUNT;
       if (i != j)
@@ -138,14 +138,14 @@
     }
 
   /* Verify that all the values are from this thread.  */
-  dbgprintf ("Worker %p before final verify\n", gl_thread_self ());
+  dbgprintf ("Worker %p before final verify\n", gl_thread_self_pointer ());
   for (i = 0; i < KEYS_COUNT; i++)
     if ((*(unsigned int *) gl_tls_get (mykeys[i]) % THREAD_COUNT) != id)
       abort ();
-  dbgprintf ("Worker %p after  final verify\n", gl_thread_self ());
+  dbgprintf ("Worker %p after  final verify\n", gl_thread_self_pointer ());
   perhaps_yield ();
 
-  dbgprintf ("Worker %p dying.\n", gl_thread_self ());
+  dbgprintf ("Worker %p dying.\n", gl_thread_self_pointer ());
   return NULL;
 }
 
-- 
In memoriam Johanna Kirchner <http://en.wikipedia.org/wiki/Johanna_Kirchner>