This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Add C11 threads.h support.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]