[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHs-p=ii962hRJ2jkcsBE1R8eVyvROwAP5zy_EwpbafzjGhimQ@mail.gmail.com>
Date: Thu, 5 Jun 2025 23:41:47 +0100
From: Andy Caldwell <andy.m.caldwell@...glemail.com>
To: musl@...ts.openwall.com
Cc: Rich Felker <dalias@...c.org>, noloader@...il.com
Subject: Re: Sign conversion warning in FD_ISSET()
On Thu, Jun 5, 2025 at 11:29 PM David Steele <david@...ackrest.org> wrote:
>
> On 6/5/25 17:50, Rich Felker wrote:
> > On Thu, Jun 05, 2025 at 09:42:27PM +0000, David Steele wrote:
> >> On 5/23/25 12:07, Jeffrey Walton wrote:
> >>> For some background, see the discussion at
> >>> <https://www.openwall.com/lists/musl/2024/07/16/1>.
> >>
> >> Well, OK, but I must say that "this is a bad warning" is not a very
> >> satisfying answer. I get that casting a pointer to int would be bad but is
> >> having the compiler do a silent implicit conversion better? To be clear, in
> >> this case I have not enabled this particular warning but instead
> >> -Wconversion, which includes it. In general I have found these warnings to
> >> be useful -- usually the warnings point out mistakes but in edge cases a
> >> cast can be added and documented so it is clear why the conversion is being
> >> done.
> >>
> >> But this is clearly not a battle that I'm going to win so I wrote a meson
> >> probe to detect the issue and disable the warning:
> >>
> >> # Suppress -Wsign-conversion for C libraries (e.g. musl) that do not accept
> >> int without warning for FD_*() macros
> >> if not cc.compiles(
> >> '''#include <sys/select.h>
> >> int main(int arg, char **argv) {int fd = 1; fd_set s; FD_ZERO(&s);
> >> FD_SET(fd, &s); FD_ISSET(fd, &s);}''',
> >> args: ['-Wsign-conversion', '-Werror'])
> >> message('Disabling -Wsign-conversion because int is not accepted without
> >> warning by FD_*() macros')
> >> add_project_arguments(cc.get_supported_arguments('-Wno-sign-conversion'),
> >> language: 'c')
> >> endif
> >>
> >> I post this in case it is of use to others who run into this issue.
> >
> > This is still in the domain of "stuff we may change", but there does
> > not seem to be a clear right way to do it without masking warnings for
> > more serious bugs like passing an actually-wrong type. If you or
> > anyone else has good ideas for a recipe that wouldn't accept pointers
> > and cast them to ints (and that would leave all constraint violations
> > as constraint violations), please share it for discussion as a
> > potential way forward.
>
> I wonder if it would be possible to use _Static_assert for this purpose,
> when it is available, i.e. cast to unsigned when the input is int.
> Something like this (but not this):
>
> #ifdef HAVE_BUILTIN_TYPES_COMPATIBLE_P
> #define UNCONSTIFY(type, expression)
> \
>
> (STATIC_ASSERT_EXPR(__builtin_types_compatible_p(__typeof(expression),
> const type), "invalid cast"), (type)(expression))
> #else
> #define UNCONSTIFY(type, expression)
> \
> ((type)(expression))
> #endif
>
> Here is a link for more context but for the sake of the list I wanted to
> present at least minimal code:
> https://github.com/pgbackrest/pgbackrest/blob/main/src/common/macro.h#L71
>
> I cribbed this from Postgres and it is handy when a library like libbz2
> does not accept const for source data that should never be changed.
>
> > In general, it looks like -Wsign-conversion is a very badly-behaved
> > warning that's not interacting right with the normal logic to suppress
> > warnings from system headers. It would be nice if the compiler side
> > could fix that.
>
> Hmmm, interesting. I wonder if this would be worth a report to gcc.
> Wouldn't be my first one. Probably best to test against the most recent
> version (or whatever is in Fedora 42) first and see if the warning is
> still there.
>
> Thanks,
> -David
You can get (most of the way) there with compound literals (C99 and later):
#define SOCKET_TO_UNSIGNED(s) (unsigned)(int[]){(s)}[0]
This accepts anything coercible to an `int` and casts it to an
`unsigned` - means you can pass in a `bool` or a `short` but it won't
accept a pointer. With `-Wsign-conversion` this will generate
warnings if the user passes an `unsigned int` but since the FD_SET
etc. macros expect to get sockets which are `int`s, that warning feels
less troubling.
Andy
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
