This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Suppress sign-conversion warning from FD_SET
- From: Paul Eggert <eggert at cs dot ucla dot edu>
- To: Paul Pluzhnikov <ppluzhnikov at google dot com>
- Cc: libc-alpha at sourceware dot org, yunlian at google dot com
- Date: Fri, 25 May 2012 14:23:40 -0700
- Subject: Re: [PATCH] Suppress sign-conversion warning from FD_SET
- References: <20120525162702.053F819076C@elbrus2.mtv.corp.google.com>
On 05/25/2012 09:27 AM, Paul Pluzhnikov wrote:
> - ({ unsigned long int __d = (d); \
> + ({ unsigned long int __d = (unsigned long int) (d); \
As a general rule I'd rather avoid unnecessary casts.
The patch does not fix a bug, as the implicit conversion from
D's type 'int' to 'unsigned long int' has well-defined
behavior. And the patch may mask a later bug if someone
mistakenly invokes the macro with D being a pointer.
Instead, how about using 'long int', since that's guaranteed
to work without munging the sign? Something like this:
#define __FD_ELT(d) \
__extension__ \
({ long int __d = (d); \
(__builtin_constant_p (__d) \
? (0 <= __d && __d < __FD_SETSIZE \
? __d / __NFDBITS : __fdelt_warn (__d)) \
: __fdelt_chk (__d)); })
with a similar change to fdelt_chk's implementation, and
with the signatures of __fdelt_warn and __fdelt_chk
changed to use 'long int' rather than 'unsigned long int'.
The generated code should be identical.