This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC] Add C11 threads.h support.
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Juan Manuel Torres Palma <j dot m dot torrespalma at gmail dot com>
- Cc: libc-alpha <libc-alpha at sourceware dot org>
- Date: Fri, 26 Jun 2015 21:22:10 +0000
- Subject: Re: [RFC] Add C11 threads.h support.
- Authentication-results: sourceware.org; auth=none
- References: <CAD82F-pXPRnsxLtT2pTADWkjsV=m6xA7cDuMX6J_bXW+c=HsVA at mail dot gmail dot com>
On Fri, 26 Jun 2015, Juan Manuel Torres Palma wrote:
> Dear glibc developers,
>
> As I am preparing what I think that could be my last patch to have C11
> threads.h working, I still have a couple doubts to be solved.
>
> First, I would like any of you to review this patch and let me know if
> my approach is correct, by separating types in bits/threadstypes.h
> from functions in include/threads.h.
I'm not sure it's useful without any targets that would define the types
differently; I think bits/ headers are only needed where there are such
target-dependencies (so bits/thread-shared-types.h is needed, but maybe
not bits/threadstypes.h).
> Second, I have noticed that there are files that I haven't updated,
> that I think that I should. Those files are libpthread.abilist, and I
> don't know how should I do it. Should I do it manually? Or is there a
> script to get the symbols from the library once it's compiled.
You should update libpthread.abilist (all copies). If you examine the
diffs from "make check-abi" and confirm they are as expected, you can
apply them to the copy of libpthread.abilist used in your configuration,
and then copy the same entries to all other copies.
The patch should also add documentation and testcases for all the
functions, as well as updating the conform/ tests to cover the new header.
> +/* Declaration of C11 threads.h types. All of the types are based
> + on pthread types to ease implementation and compatibility.
> + Most of these types can be consulted in Linux Standard Base
> + Specifications. */
I don't think LSB should be referred to in glibc headers. Also note ". "
(dot, two spaces) at the end of comments.
> +#ifndef __STDC_NO_THREADS__
No such conditional needed. __STDC_NO_THREADS__ should never be defined
once this support is in.
> +//#if ( defined __STDC_VERSION__ && __STDC_VERSION__ < 201112L )
> +//# error "<threads.h> requires ISO C11 mode"
> +//#else
Don't include such commented-out code.
> +# ifdef __cplusplus
> +__BEGIN_DECLS
> +# endif
No __cplusplus here, since __BEGIN_DECLS / __END_DECLS are defined to
empty for non-C++.
> +/* Threads functions */
> +extern int thrd_create (thrd_t *thr, thrd_start_t func, void *arg );
Each function should have a comment explaining its semantics in the
header. Argument names need to be in the implementation namespace (the
conform/ tests will tell you if you get this wrong, once you've added
threads.h data to them). Stray space before ')'.
> +void call_once (once_flag *flag, void (*func)(void))
Bad formatting for function definition. The return type goes on the line
before the name, and there should be a comment explaining the semantics of
the function and its arguments.
> +{
> + pthread_once (flag, func);
You need to call the implementation-namespace names such as
__pthread_once. It's not enough that they exist if you then use the other
names for those functions; you must avoid any strong references (defined
or undefined) to names outside the C11 and implementation namespaces (and
any uses of weak undefined symbols outside those namespaces that might get
executed for conforming code).
--
Joseph S. Myers
joseph@codesourcery.com