Re: [PATCH] Clean pthread types namespaces on x86 and x86_64

[Juan Manuel Torres Palma <j dot m dot torrespalma at gmail dot com>]

> I don't think the short cryptic file names are good.  I'd prefer something
> like bits/thread-shared-types.h for the content of types that are shared
> between pthreads and C11 threads, and bits/pthreadtypes-common.h for the
> definitions of pthread_cond_t and pthread_mutex_t in terms of that type
> content.

I agree with you, but some other people prefer short names. I will
change file names to more specific ones.

> Mechanical patches such as this one are expected to update all
> architectures to keep the sources bisectable.  That includes creating
> dummy versions of the new headers in the toplevel bits/, like the existing
> bits/pthreadtypes.h, as well as versions of bits/pthread_st.h (which I
> think should be bits/thread-shared-types.h) for each architecture that
> currently has its own bits/pthreadtypes.h,

I planned to update every architecture with a different patch better
than a single huge one, so it could be tracked down if any issue
happens since I can only test for x86_64 and ARM. Or do you mean
creating bits/thread-shared-types.h so other architectures can still
build? I didn't realize of this last possibility until now...

Cheers.

2015-06-23 22:20 GMT+02:00 Joseph Myers <joseph@codesourcery.com>:
> On Tue, 23 Jun 2015, Juan Manuel Torres Palma wrote:
>
>> This patch creates the file bits/pthread_ct.h which contains types
>> pthread_mutex_t and pthread_cond_t, that were previously in
>> pthreadtypes.h. The actual structure declaration
>> is in pthread_st.h, that stands for specific types, so now when
>> creating mtx_t and cnd_t structs, namespaces won't be corrupted.
>
> I don't think the short cryptic file names are good.  I'd prefer something
> like bits/thread-shared-types.h for the content of types that are shared
> between pthreads and C11 threads, and bits/pthreadtypes-common.h for the
> definitions of pthread_cond_t and pthread_mutex_t in terms of that type
> content.
>
> Mechanical patches such as this one are expected to update all
> architectures to keep the sources bisectable.  That includes creating
> dummy versions of the new headers in the toplevel bits/, like the existing
> bits/pthreadtypes.h, as well as versions of bits/pthread_st.h (which I
> think should be bits/thread-shared-types.h) for each architecture that
> currently has its own bits/pthreadtypes.h,
>
>> +typedef union
>> +{
>> +     __PTHREAD_MUTEX_T_CONTENT
>
> The indentation looks odd here (and in several other places) - it should
> be two spaces, not a tab (with initial multiples of 8 spaces replaced by
> tabs).
>
> --
> Joseph S. Myers
> joseph@codesourcery.com



-- 
Juan Manuel Torres Palma.
Computer Science Student at Universidad de Granada.