[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3b1f8664-725b-46b4-b26c-f91d4a92bbce@brad-house.com>
Date: Thu, 5 Jun 2025 19:38:33 -0400
From: Brad House <brad@...d-house.com>
To: musl@...ts.openwall.com, Rich Felker <dalias@...c.org>,
David Steele <david@...ackrest.org>
Cc: noloader@...il.com
Subject: Re: Sign conversion warning in FD_ISSET()
On 6/5/25 7:31 PM, Rich Felker wrote:
> On Thu, Jun 05, 2025 at 10:26:21PM +0000, David Steele 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
> Any kind of GNUC monstrosity like this is a non-starter. Proposals
> need to actually be C. And C89-compatible, since the header can be
> used in C89 mode.
>
> There probably is some way to get it right with clever use of the
> ternary operator but I haven't come up with anything in the first few
> seconds thinking about it.
>
> Rich
Hey guys, I tried to address this a while back. This thread may be
useful (linking to last post in thread):
https://www.openwall.com/lists/musl/2024/08/19/2
Kind of stalled out as I really wanted to sanity check FD_SETSIZE like
bionic does but never got a reply.
-Brad
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.
