diff mbox

[RFC] Fixing pthread_* namespace issues for thrd_* symbols

Message ID CAD82F-qNPdSqDE2d+aBm=jUp5TfJHw3fzo2bYb0KasDv6WFxwQ@mail.gmail.com
State New
Headers show

Commit Message

Juan Manuel Torres Palma June 19, 2015, 9:02 a.m. UTC 
2015-06-18 16:12 GMT+02:00 Joseph Myers <joseph@codesourcery.com>:

> Contents shouldn't be duplicated, but you could e.g. have a shared header
> that defines macros such as __PTHREAD_COND_T_CONTENT, so cnd_t would be
>
> typedef union
> {
>   __PTHREAD_COND_T_CONTENT
> } cnd_t;
>
> and pthread_cond_t similarly.

I have done that, but still have some code duplication (different
mutex types for x86_64 and x86) but I think is acceptable because the
code is easy to read. I could fix it splitting it in several macros,
but I think it will look a bit more messy. However I will do it if
needed, I'm still learning :)

Here is the last patch.

Cheers.

---------------------------------------------------------------------------------------------------------------------------------------

From 96720c367d92139a0cca5eef6deccb0744e2b92e Mon Sep 17 00:00:00 2001
From: Juan Manuel Torres Palma <jmtorrespalma@gmail.com>
Date: Fri, 19 Jun 2015 10:50:33 +0200
Subject: [PATCH] Second solution to fix C11 types

---
 posix/Makefile                        |   2 +-
 sysdeps/x86/bits/pthreadsharedtypes.h | 131 ++++++++++++++++++++++++++++++++++
 sysdeps/x86/bits/pthreadtypes.h       |  72 +------------------
 sysdeps/x86/bits/threadstypes.h       |  41 +++++++++++
 4 files changed, 176 insertions(+), 70 deletions(-)
 create mode 100644 sysdeps/x86/bits/pthreadsharedtypes.h
 create mode 100644 sysdeps/x86/bits/threadstypes.h

Comments

Joseph Myers June 19, 2015, 5 p.m. UTC | #1 
On Fri, 19 Jun 2015, Juan Manuel Torres Palma wrote:

> 2015-06-18 16:12 GMT+02:00 Joseph Myers <joseph@codesourcery.com>:
> 
> > Contents shouldn't be duplicated, but you could e.g. have a shared header
> > that defines macros such as __PTHREAD_COND_T_CONTENT, so cnd_t would be
> >
> > typedef union
> > {
> >   __PTHREAD_COND_T_CONTENT
> > } cnd_t;
> >
> > and pthread_cond_t similarly.
> 
> I have done that, but still have some code duplication (different
> mutex types for x86_64 and x86) but I think is acceptable because the
> code is easy to read. I could fix it splitting it in several macros,
> but I think it will look a bit more messy. However I will do it if
> needed, I'm still learning :)

This is looking closer to what I'd expect if we do go with the same layout 
of C11 and POSIX threads types (subject of course to updating all 
architectures similarly).  Now, I don't see any reason for 
bits/threadstypes.h to be architecture-specific.  Furthermore, maybe the 
parts of bits/pthreadtypes.h that now just use the *_CONTENT macros 
shouldn't be architecture-specific either (that is, they should be in a 
new bits/ header with only one version of that header in the tree).
diff mbox

Patch

diff --git a/posix/Makefile b/posix/Makefile
index 15e8818..32d4146 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -29,7 +29,7 @@  headers := sys/utsname.h sys/times.h sys/wait.h
sys/types.h unistd.h      \
    bits/local_lim.h tar.h bits/utsname.h bits/confname.h      \
    bits/waitflags.h bits/waitstatus.h sys/unistd.h sched.h      \
    bits/sched.h re_comp.h wait.h bits/environments.h cpio.h      \
-   sys/sysmacros.h spawn.h bits/unistd.h
+   sys/sysmacros.h spawn.h bits/unistd.h bits/pthreadsharedtypes.h

 routines :=      \
  uname      \
diff --git a/sysdeps/x86/bits/pthreadsharedtypes.h
b/sysdeps/x86/bits/pthreadsharedtypes.h
new file mode 100644
index 0000000..d9454ec
--- /dev/null
+++ b/sysdeps/x86/bits/pthreadsharedtypes.h
@@ -0,0 +1,131 @@ 
+/* Copyright (C) 2002-2015 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/>.  */
+
+/* This header contains the declaration of pthread_mutex_t and
+   pthread_cond_t needed by C11 types mtx_t and cnd_t. */
+
+#ifndef _BITS_PTHREADSHAREDTYPES_H
+#define _BITS_PTHREADSHAREDTYPES_H 1
+
+#include <bits/wordsize.h>
+
+#ifdef __x86_64__
+# if __WORDSIZE == 64
+#  define __SIZEOF_PTHREAD_MUTEX_T 40
+#  define __SIZEOF_PTHREAD_COND_T 48
+# else
+#  define __SIZEOF_PTHREAD_MUTEX_T 32
+#  define __SIZEOF_PTHREAD_COND_T 48
+# endif
+#else
+# define __SIZEOF_PTHREAD_MUTEX_T 24
+# define __SIZEOF_PTHREAD_COND_T 48
+#endif
+
+#ifdef __x86_64__
+typedef struct __pthread_internal_list
+{
+  struct __pthread_internal_list *__prev;
+  struct __pthread_internal_list *__next;
+} __pthread_list_t;
+#else
+typedef struct __pthread_internal_slist
+{
+  struct __pthread_internal_slist *__next;
+} __pthread_slist_t;
+#endif
+
+
+/* Data structure for mutex handling. */
+
+#ifdef __x86_64__
+
+# define __PTHREAD_MUTEX_HAVE_PREV 1
+/* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER.  */
+# define __PTHREAD_SPINS             0, 0
+
+
+# define __PTHREAD_MUTEX_T_CONTENT            \
+  struct __pthread_mutex_s                    \
+  {                                           \
+    int __lock;                               \
+    unsigned int __count;                     \
+    int __owner;                              \
+    unsigned int __nusers;                    \
+    /* KIND must stay at this position in the structure to maintain
+       binary compatibility.  */              \
+    int __kind;                               \
+    short __spins;                            \
+    short __elision;                          \
+    __pthread_list_t __list;                  \
+  } __data;                                   \
+  char __size[__SIZEOF_PTHREAD_MUTEX_T];      \
+  long int __align;
+
+
+
+#else
+
+# define __spins __elision_data.__espins
+# define __elision __elision_data.__elision
+# define __PTHREAD_SPINS         { 0, 0 }
+
+
+# define __PTHREAD_MUTEX_T_CONTENT         \
+  struct pthread_mutex_s                   \
+  {                                        \
+    int __lock;                            \
+    unsigned int __count;                  \
+    int __owner;                           \
+    /* KIND must stay at this position. */ \
+    int __kind;                            \
+    unsigned int __nusers;                 \
+    __extension__ union                    \
+    {                                      \
+      struct                               \
+      {                                    \
+ short __espins;                        \
+ short __elision;                       \
+      } __elision_data;                    \
+      __pthread_slist_t __list;            \
+    };                                     \
+  } __data;                                \
+  char __size[__SIZEOF_PTHREAD_MUTEX_T];   \
+  long int __align;
+
+#endif
+
+
+/* Data structure for conditional variable handling */
+
+#define __PTHREAD_COND_T_CONTENT                          \
+  struct                                                  \
+  {                                                       \
+    int __lock;                                           \
+    unsigned int __futex;                                 \
+    __extension__ unsigned long long int __total_seq;     \
+    __extension__ unsigned long long int __wakeup_seq;    \
+    __extension__ unsigned long long int __woken_seq;     \
+    void *__mutex;                                        \
+    unsigned int __nwaiters;                              \
+    unsigned int __broadcast_seq;                         \
+  } __data;                                               \
+  char __size[__SIZEOF_PTHREAD_COND_T];                   \
+  __extension__ long long int __align;
+
+
+#endif /* bits/pthreadsharedtypes.h */
diff --git a/sysdeps/x86/bits/pthreadtypes.h b/sysdeps/x86/bits/pthreadtypes.h
index 4460615..f900cef 100644
--- a/sysdeps/x86/bits/pthreadtypes.h
+++ b/sysdeps/x86/bits/pthreadtypes.h
@@ -19,13 +19,12 @@ 
 #define _BITS_PTHREADTYPES_H 1

 #include <bits/wordsize.h>
+#include <bits/pthreadsharedtypes.h>

 #ifdef __x86_64__
 # if __WORDSIZE == 64
 #  define __SIZEOF_PTHREAD_ATTR_T 56
-#  define __SIZEOF_PTHREAD_MUTEX_T 40
 #  define __SIZEOF_PTHREAD_MUTEXATTR_T 4
-#  define __SIZEOF_PTHREAD_COND_T 48
 #  define __SIZEOF_PTHREAD_CONDATTR_T 4
 #  define __SIZEOF_PTHREAD_RWLOCK_T 56
 #  define __SIZEOF_PTHREAD_RWLOCKATTR_T 8
@@ -33,9 +32,7 @@ 
 #  define __SIZEOF_PTHREAD_BARRIERATTR_T 4
 # else
 #  define __SIZEOF_PTHREAD_ATTR_T 32
-#  define __SIZEOF_PTHREAD_MUTEX_T 32
 #  define __SIZEOF_PTHREAD_MUTEXATTR_T 4
-#  define __SIZEOF_PTHREAD_COND_T 48
 #  define __SIZEOF_PTHREAD_CONDATTR_T 4
 #  define __SIZEOF_PTHREAD_RWLOCK_T 44
 #  define __SIZEOF_PTHREAD_RWLOCKATTR_T 8
@@ -44,9 +41,7 @@ 
 # endif
 #else
 # define __SIZEOF_PTHREAD_ATTR_T 36
-# define __SIZEOF_PTHREAD_MUTEX_T 24
 # define __SIZEOF_PTHREAD_MUTEXATTR_T 4
-# define __SIZEOF_PTHREAD_COND_T 48
 # define __SIZEOF_PTHREAD_CONDATTR_T 4
 # define __SIZEOF_PTHREAD_RWLOCK_T 32
 # define __SIZEOF_PTHREAD_RWLOCKATTR_T 8
@@ -71,60 +66,11 @@  typedef union pthread_attr_t pthread_attr_t;
 #endif


-#ifdef __x86_64__
-typedef struct __pthread_internal_list
-{
-  struct __pthread_internal_list *__prev;
-  struct __pthread_internal_list *__next;
-} __pthread_list_t;
-#else
-typedef struct __pthread_internal_slist
-{
-  struct __pthread_internal_slist *__next;
-} __pthread_slist_t;
-#endif
-
-
 /* Data structures for mutex handling.  The structure of the attribute
    type is not exposed on purpose.  */
 typedef union
 {
-  struct __pthread_mutex_s
-  {
-    int __lock;
-    unsigned int __count;
-    int __owner;
-#ifdef __x86_64__
-    unsigned int __nusers;
-#endif
-    /* KIND must stay at this position in the structure to maintain
-       binary compatibility.  */
-    int __kind;
-#ifdef __x86_64__
-    short __spins;
-    short __elision;
-    __pthread_list_t __list;
-# define __PTHREAD_MUTEX_HAVE_PREV 1
-/* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER.  */
-# define __PTHREAD_SPINS             0, 0
-#else
-    unsigned int __nusers;
-    __extension__ union
-    {
-      struct
-      {
- short __espins;
- short __elision;
-# define __spins __elision_data.__espins
-# define __elision __elision_data.__elision
-# define __PTHREAD_SPINS         { 0, 0 }
-      } __elision_data;
-      __pthread_slist_t __list;
-    };
-#endif
-  } __data;
-  char __size[__SIZEOF_PTHREAD_MUTEX_T];
-  long int __align;
+ __PTHREAD_MUTEX_T_CONTENT
 } pthread_mutex_t;

 typedef union
@@ -138,19 +84,7 @@  typedef union
    the attribute type is not exposed on purpose.  */
 typedef union
 {
-  struct
-  {
-    int __lock;
-    unsigned int __futex;
-    __extension__ unsigned long long int __total_seq;
-    __extension__ unsigned long long int __wakeup_seq;
-    __extension__ unsigned long long int __woken_seq;
-    void *__mutex;
-    unsigned int __nwaiters;
-    unsigned int __broadcast_seq;
-  } __data;
-  char __size[__SIZEOF_PTHREAD_COND_T];
-  __extension__ long long int __align;
+ __PTHREAD_COND_T_CONTENT
 } pthread_cond_t;

 typedef union
diff --git a/sysdeps/x86/bits/threadstypes.h b/sysdeps/x86/bits/threadstypes.h
new file mode 100644
index 0000000..6ae5e0f
--- /dev/null
+++ b/sysdeps/x86/bits/threadstypes.h
@@ -0,0 +1,41 @@ 
+/* Copyright (C) 2002-2015 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/>.  */
+
+/* Contains declaration of C11 types: mtx_t and cnd_t. */
+
+
+#ifndef _BITS_THREADSTYPES_H
+#define _BITS_THREADSTYPES_H 1
+
+#include <bits/pthreadsharedtypes.h>
+
+/* mtx_t is a copy of pthread_mutex_t. */
+
+typedef union
+{
+ __PTHREAD_MUTEX_T_CONTENT
+} mtx_t;
+
+
+/* cnd_t is a copy of pthread_cond_t. */
+
+typedef union
+{
+ __PTHREAD_COND_T_CONTENT
+} cnd_t;
+
+#endif /* bits/threadstypes.h */