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

From: Jeff Law
Date: Tue Jul 24 2012 - 15:15:56 EST


On 07/24/12 13:09, Linus Torvalds wrote:
On Tue, Jul 24, 2012 at 12:03 PM, Josh Boyer <jwboyer@xxxxxxxxxx> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/