[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c1e05585-3a2e-495f-bd5f-7fbf4005f974@pgbackrest.org>
Date: Thu, 05 Jun 2025 22:26:21 +0000 (UTC)
From: David Steele <david@...ackrest.org>
To: Rich Felker <dalias@...c.org>
Cc: noloader@...il.com, musl@...ts.openwall.com
Subject: Re: Sign conversion warning in FD_ISSET()
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
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.
