This is the mail archive of the cygwin-patches mailing list for the Cygwin 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: [PATCH] POSIX barrier implementation, take 2


On 12.2.2016 15:25, Corinna Vinschen wrote:
> Hi VÃclav,
> 
> 
> the patch looks pretty good, I have just a few (minor) nits:
> 
> On Feb 12 11:20, VÃclav Haisman wrote:
>> diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h
>> index 5dd6c75..bfe93fa 100644
>> --- a/newlib/libc/include/sys/types.h
>> +++ b/newlib/libc/include/sys/types.h
>> @@ -431,6 +431,7 @@ typedef struct {
>>  
>>  /* POSIX Barrier Types */
>>  
>> +#if !defined(__CYGWIN__)
>>  #if defined(_POSIX_BARRIERS)
>>  typedef __uint32_t pthread_barrier_t;        /* POSIX Barrier Object */
>>  typedef struct {
>> @@ -440,6 +441,7 @@ typedef struct {
>>  #endif
>>  } pthread_barrierattr_t;
>>  #endif /* defined(_POSIX_BARRIERS) */
>> +#endif /* __CYGWIN__ */
> 
> Instead of adding YA `if !CYGWIN', I think it might be prudent to
> just move the `if !CYGWIN' up from the following _POSIX_SPIN_LOCKS
> block.

OK, will do.

> 
>> diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
>> index d7f4d24..18e010a 100644
>> --- a/winsup/cygwin/common.din
>> +++ b/winsup/cygwin/common.din
>> @@ -882,6 +882,13 @@ pthread_condattr_getpshared SIGFE
>>  pthread_condattr_init SIGFE
>>  pthread_condattr_setclock SIGFE
>>  pthread_condattr_setpshared SIGFE
>> +pthread_barrierattr_init SIGFE
>> +pthread_barrierattr_setpshared SIGFE
>> +pthread_barrierattr_getpshared SIGFE
>> +pthread_barrierattr_destroy SIGFE
>> +pthread_barrier_init SIGFE
>> +pthread_barrier_destroy SIGFE
>> +pthread_barrier_wait SIGFE
>>  pthread_continue SIGFE
>>  pthread_create SIGFE
>>  pthread_detach SIGFE
> 
> These should be added in alphabetic order.

OK. I did not realize that.

> 
>> +#define LIKELY(X) __builtin_expect (!!(X), 1)
>> +#define UNLIKELY(X) __builtin_expect (!!(X), 0)
> 
> May I suggest to use lowercase "likely/unlikely" just as in the Linux
> kernel and to move the definitions into a header like winsup.h or
> miscfuncs.h?

I was not sure where to put these and then I forgot. Will fix.


-- 
VH

Attachment: signature.asc
Description: OpenPGP digital signature


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