Re: [PATCH v4] fix warnings in sys/select.h fortification with -Wsign-conversion

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Sat, Jun 9, 2012 at 3:51 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> If we build code such as:
> ? ? ? ?#include <sys/select.h>
> ? ? ? ?int main(int argc, char *argv[])
> ? ? ? ?{
> ? ? ? ? ? ? ? ?int fd = argc;
> ? ? ? ? ? ? ? ?fd_set fds;
> ? ? ? ? ? ? ? ?FD_ZERO(&fds);
> ? ? ? ? ? ? ? ?FD_SET(fd, &fds);
> ? ? ? ? ? ? ? ?return argv[0][fd];
> ? ? ? ?}
>
> with warning flags such as:
> ? ? ? ?$ gcc test.c -c -Wsign-conversion -O2 -Werror -D_FORTIFY_SOURCE=2
>
> the build fails like so:
> ? ? ? ?error: conversion to 'long unsigned int' from 'int' may change
> ? ? ? ? ? ? ? ?the sign of the result [-Werror=sign-conversion]
>
> This is because the fortification code is casting the signed fd up to
> an unsigned value. ?If the fd is actually negative (causing the unsigned
> value to cast to a large number), the following check in the code will
> catch it and warn properly.
>
> So add an explicit cast to avoid the warning.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> 2012-06-08 ?Mike Frysinger ?<vapier@gentoo.org>
>
> ? ? ? ?[BZ #14210]
> ? ? ? ?* misc/bits/select2.h (__FD_ELT): Cast D to unsigned long int.
> ---
> v4
> ? ? ? ?- tweak new comment from Andreas Schwab
>
> ?misc/bits/select2.h | ? ?3 ++-
> ?1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/misc/bits/select2.h b/misc/bits/select2.h
> index 9679925..db04454 100644
> --- a/misc/bits/select2.h
> +++ b/misc/bits/select2.h
> @@ -27,7 +27,8 @@ extern unsigned long int __fdelt_warn (unsigned long int __d)
> ?#undef __FD_ELT
> ?#define ? ? ? ?__FD_ELT(d) \
> ? __extension__ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> - ?({ unsigned long int __d = (d); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ?({ /* Avoid a warning from -Wsign-conversion. ?*/ ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? unsigned long int __d = (unsigned long int) (d); ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ?(__builtin_constant_p (__d) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? (__d >= __FD_SETSIZE ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? __fdelt_warn (__d) : (__d / __NFDBITS)) ? ? ? ? ? ? ? ? ? ? ? ? ?\
> --
> 1.7.9.7
>

If nobody else objects please check this in on Monday.

Truth be told this is *exactly* the kind of issue I want fixed during
the freeze.

I'm still worried about the other _FORTIFY_SOURCE issues you found.
Those are the kinds of things that make the community look out of
touch with regards to compiling the large bodies of source that exist.
We need to eat our own dog-food and try to build as much user code as
possible during the freeze.

I'm very very happy to see you and Allan taking it seriously and
building user code with the proposed 2.16.

Cheers,
Carlos.