svn commit: r301071 - head/sys/sys

[Bruce Evans]

On Wed, 1 Jun 2016, Konstantin Belousov wrote:

> On Wed, Jun 01, 2016 at 05:44:47PM +0200, Ed Schouten wrote:
>>  ...
>>>>  - In our implementation, struct sigevent::sigev_notify_attributes has
>>>>    type "void *" instead of "pthread_attr_t *". My guess is that this was
>>>>    done to prevent pulling in the pthread types, but this can easily be
>>>>    avoided by using the underlying structure types.
>>>
>>> Not easily, since the tags of the underlying struct types are in the
>>> application namespace, at least up to POSIX 2001.
>>
>> Yeah, it's quite unfortunate that we use structure types starting with
>> 'pthread'. They should have had leading underscores. But in my opinion
>> that's not a problem specific to this change; it's a problem with our
>> pthread implementation in general.
>>
>>>> +#include <sys/_pthreadtypes.h>

Indeed.  The problem became larger in r146824 when these types started
to be declared unconditionally in <sys/types.h>.

Another bug in sys/_pthreadtypes.h is that it says that the prefixes
pthread_ and PTHREAD_ are reserved for use in header symbols.  Actually,
they are only reserved for use in <pthread.h>.  This shouldn't be documented
in the general header.  The non-broken parts of general header depend on
symbols ending with _t being reserved, not on this.

>>> This gives the following pollution (which breaks almost everything since
>>> <sys/types.h> includes this header:
>>> - struct tag names pthread*
>>> - struct member names state and mutex
>>
>> Yes. It would have made so much more sense if a header like
>> <sys/_types.h> would have defined all pthread types as __pthread_t,
>> __pthread_mutex_t, etc. That way there would have been a way to expose
>> just pthread_t and pthread_attr_t without pulling in the rest.
> No, it wouldn't.

Putting everything in sys/_types.h (or machine/_types.h) is convenient,
and it avoids proliferation of headers, but it gives a different bloat
problem: every header ends up declaring every type, but with an spelling
in the implementation namespace.  Then the number of types almost doubles
when you declare only almost all types again in the application namespace.

> Replace the typedefs with the forward-struct names by the void *.
> The only other change would be the libthr, where some casts might
> be needed.
>
> Use void * directly in signal.h if possible.

Pointers to incomplete structures give more type safety than void *.
Except for the problem with struct tags, just using them is best.  Unlike
for typedefs, we don't need a 5-line ifdef for every use to avoid
redeclaratiion errors.

sys/signal.h seems to use a pthread type just once.  This type can be
declared as 'struct pthread_attr *'.

Actually, signal.h already used the correct method, except this was
obfuscated using a private typedef (__pthread_t), and at some point
when POSIX apparently started explicitly requiring <signal.h> to
declare pthread_t, <signal.h> was polluted to match.

We use ifdefs to a fault to minimise the scope of typedefs like size_t.
Not doing this for the pthread types is inconsistent.  For POSIX headers,
it is easiest to declare all typedefs *_t in all headers.  This may
actually be more efficient by avoiding 1 layer of indirect typedefs like
__size_t and 2-3 layers of header convolutions and ifdefs.  The
indirections and convolutuons are still technically needed for non-POSIX
headers.

Bruce