[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140907002123.GX23797@brightrain.aerifal.cx>
Date: Sat, 6 Sep 2014 20:21:23 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/9] interface additions for the C thread
implementation
On Mon, Sep 01, 2014 at 12:45:47AM +0200, Jens Gustedt wrote:
> This adds all the constant, type and function interfaces.
Including some comments here as I work on committing it. Please don't
take them as critical; just explaining choices made during commit and
noting where things are different in the version to be committed.
> It makes pthread_mutex_t, mtx_t, pthread_cond_t and cnd_t different
> types.
>
> This only works because
>
> - under hood the corresponding pairs of types use exactly the same
> definition for the type
>
> - the types are a struct types without tag name
>
> - no comparison or assignment is allowed for any of these types. For
> the POSIX types this interdiction is written in the standard. For the
> C thread types, this is an extension that this implementation
> imposes, but which might be integrated in a later version of the C
> standard.
>
> - initialization is default initialization of an array of int. For the
> POSIX types, initialization expressions are provided. For C thread
> types the only initialization foreseen by the standard are the init
> functions.
>
> - any calls to standard functions use pointers, and because pointer
> representations for struct types are the same.
>
> For the C++ API/ABI, these also are different types, now, with type names
> (that are used for name mangling, e.g) as listed above.
>
> Somebody better versed in C++ could perhaps contribute code that
> overloads the comparison and assignment operators such that a compilation
> that tries to compare or copy these types fails.
I'm not sure what you meant by this last paragraph.
> ---
> arch/arm/bits/alltypes.h.in | 10 +++-
> arch/i386/bits/alltypes.h.in | 10 +++-
> arch/microblaze/bits/alltypes.h.in | 10 +++-
> arch/mips/bits/alltypes.h.in | 10 +++-
> arch/or1k/bits/alltypes.h.in | 10 +++-
> arch/powerpc/bits/alltypes.h.in | 10 +++-
> arch/sh/bits/alltypes.h.in | 10 +++-
> arch/x32/bits/alltypes.h.in | 10 +++-
> arch/x86_64/bits/alltypes.h.in | 10 +++-
> include/alltypes.h.in | 10 ++++
> include/threads.h | 110 ++++++++++++++++++++++++++++++++++++
> include/time.h | 11 ++++
> 12 files changed, 212 insertions(+), 9 deletions(-)
> create mode 100644 include/threads.h
>
> diff --git a/arch/arm/bits/alltypes.h.in b/arch/arm/bits/alltypes.h.in
> index 183c4c4..1dcb920 100644
> --- a/arch/arm/bits/alltypes.h.in
> +++ b/arch/arm/bits/alltypes.h.in
> @@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
> TYPEDEF long time_t;
> TYPEDEF long suseconds_t;
>
> -TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
> +/* The pairs of equivalent definitions for pthread and C thread types
> + * should always be kept in sync.
> + *
> + * Also this only works because the underlying struct has no struct
> + * tag. Don't introduce one. */
No need for comments like these; introducing a tag would be an ABI
change anyway, so it can't be done.
> diff --git a/include/alltypes.h.in b/include/alltypes.h.in
> index c4ca5d5..94364e8 100644
> --- a/include/alltypes.h.in
> +++ b/include/alltypes.h.in
> @@ -43,13 +43,23 @@ TYPEDEF unsigned gid_t;
> TYPEDEF int key_t;
> TYPEDEF unsigned useconds_t;
>
> +/* The pairs of equivalent definitions for pthread and C thread types
> + * should always be kept in sync. */
> #ifdef __cplusplus
> TYPEDEF unsigned long pthread_t;
> +TYPEDEF unsigned long thrd_t;
> #else
> TYPEDEF struct __pthread * pthread_t;
> +TYPEDEF struct __pthread * thrd_t;
> #endif
> TYPEDEF int pthread_once_t;
> +TYPEDEF int once_flag;
> TYPEDEF unsigned pthread_key_t;
> +TYPEDEF unsigned tss_t;
The threads.h types don't need to be in alltypes.h since they're only
exposed by one file. (The pthread types are there because sys/types.h
exposes them too, in addition to pthread.h.)
> +TYPEDEF pthread_cond_t cnd_t;
> +TYPEDEF pthread_mutex_t mtx_t;
These two lines were left over and seem to have been nops due to the
order of concatenation.
> diff --git a/include/threads.h b/include/threads.h
> new file mode 100644
> index 0000000..0e99443
> --- /dev/null
> +++ b/include/threads.h
> @@ -0,0 +1,110 @@
> +#ifndef _THREADS_H
> +#define _THREADS_H
> +
> +/* This one is explicitly allowed to be included. */
> +#include <time.h>
I'm also adding features.h here because musl needs it internally for
any header that wants to use restrict (we use __restrict because gcc
disables restrict by default in its ugly gnu89 mode) and _Noreturn.
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
This appeared twice with only one closing "}". I'm merging all the C++
logic into one conditional.
> +
> +/* These may or may not be implemented to be the same as some POSIX
> + * types. Don't rely on any assumption about that, in particular if
> + * you happen to use both interfaces in the same code. */
> +
This text (or similar) should eventually make it into the
documentation, but headers don't carry documentation like this in
musl.
> +int cnd_timedwait(cnd_t *restrict, mtx_t *restrict, const struct timespec *restrict);
> +int cnd_wait(cnd_t *, mtx_t *);
The lack of restrict on cnd_wait is really odd, but I checked and it's
like that in the standard too... File this as another bug for C11...
:-)
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
