Re: [PATCH v2] posix_types.h: make __NFDBITS compatible with glibc definition

[Jeff Law <law@redhat.com>]

On 07/24/12 13:09, Linus Torvalds wrote:
> On Tue, Jul 24, 2012 at 12:03 PM, Josh Boyer <jwboyer@redhat.com> wrote:
>>
>> FWIW, the definitions of __FD_ELT/__FD_MASK in glibc are:
>>
>> #define __FD_ELT(d)     ((d) / __NFDBITS)
>> #define __FD_MASK(d)    ((__fd_mask) 1 << ((d) % __NFDBITS))
>>
>> where __fd_mask is 'typdef long int'.
>
> Yeah, that's not good.
>
> If __NFDBITS is signed (and it is), and "d" is a signed type, that
> division and modulus now create stupid extra code with conditionals
> (assuming 'd' isn't constant, of course).
>
> So changing the sign of __NFDBITS has these kinds of subtle side
> effects that clearly the glibc people didn't actually think about.
>
> What was the *advantage* of that stupidity?
>
> Quite frankly, if you want to make NFDBITS be an "int", then it should
> have been done at that
>
>     #define NFDBITS ((int)__NFDBITS)
>
> level, not at "__NFDBITS". Exactly because the unsigned type there matters.
>
> Does anybody in the glibc camp care about efficient and small code AT ALL?
Please refer to the original discussion where they did evaluate the cost 
of this change and tested that the final change made no difference to 
the generated code.

http://sourceware.org/bugzilla/show_bug.cgi?id=14210

Needlessly slamming these folks doesn't help anything.

Jeff