From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [PATCH 1/9] interface additions for the C thread implementation
Date: Sat, 6 Sep 2014 20:21:23 -0400 [thread overview]
Message-ID: <20140907002123.GX23797@brightrain.aerifal.cx> (raw)
In-Reply-To: <cb03e88ca7f717c106c0aa7283202fd2f7c7f327.1409524413.git.Jens.Gustedt@inria.fr>
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
next prev parent reply other threads:[~2014-09-07 0:21 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
2014-08-31 22:45 ` [PATCH 1/9] interface additions for the C thread implementation Jens Gustedt
2014-09-07 0:21 ` Rich Felker [this message]
2014-09-07 9:13 ` Jens Gustedt
2014-09-07 10:05 ` Alexander Monakov
2014-09-07 11:16 ` Jens Gustedt
2014-09-07 11:31 ` Alexander Monakov
2014-09-07 11:32 ` Rich Felker
2014-09-07 14:45 ` Jens Gustedt
2014-09-07 15:16 ` Rich Felker
2014-09-07 16:51 ` Jens Gustedt
2014-09-07 16:55 ` Rich Felker
2014-09-07 1:19 ` Rich Felker
2014-08-31 22:46 ` [PATCH 2/9] additions to src/time and some implied minor changes here and there Jens Gustedt
2014-09-06 17:44 ` Rich Felker
2014-08-31 22:46 ` [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads Jens Gustedt
2014-09-06 18:52 ` Rich Felker
2014-08-31 22:46 ` [PATCH 4/9] add the functions for tss_t and once_flag Jens Gustedt
2014-08-31 22:46 ` [PATCH 5/9] add the functions for mtx_t Jens Gustedt
2014-09-07 1:51 ` Rich Felker
2014-09-07 1:54 ` Rich Felker
2014-08-31 22:47 ` [PATCH 6/9] add the functions for cnd_t Jens Gustedt
2014-08-31 22:47 ` [PATCH 7/9] add the thrd_xxxxxx functions Jens Gustedt
2014-09-07 14:24 ` Rich Felker
2014-09-07 14:52 ` Jens Gustedt
2014-09-07 15:17 ` Rich Felker
2014-08-31 22:47 ` [PATCH 8/9] separate pthread_create and pthread_exit in two different TU Jens Gustedt
2014-08-31 22:48 ` [PATCH 9/9] Separate pthread_create and thrd_create Jens Gustedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140907002123.GX23797@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).