[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <075e9096-7c19-41b6-b47f-87621889927d@pgbackrest.org>
Date: Thu, 05 Jun 2025 21:42:27 +0000 (UTC)
From: David Steele <david@...ackrest.org>
To: noloader@...il.com, musl@...ts.openwall.com
Subject: Re: Sign conversion warning in FD_ISSET()
On 5/23/25 12:07, Jeffrey Walton wrote:
> On Fri, May 23, 2025 at 11:31 AM David Steele <david@...ackrest.org> wrote:
>>
>> Greetings,
>>
>> I'm getting a warning when compiling on Alpine 3.21 with musl libc 1.2.5:
>>
>> # /lib/libc.musl-aarch64.so.1
>> musl libc (aarch64)
>> Version 1.2.5
>>
>> The code looks like this (simplified for this example, see original at
>> (https://github.com/pgbackrest/pgbackrest/blob/release/2.55.1/src/protocol/parallel.c#L163):
>>
>> int fd = getFdFunc();
>>
>> if (FD_ISSET(fd, &selectSet))
>>
>> And I get this warning when -Wsign-conversion is enabled on gcc:
>>
>> ../../pgbackrest/src/protocol/parallel.c:164:25: error: conversion to
>> 'long unsigned int' from 'int' may change the sign of the result
>> [-Werror=sign-conversion]
>> 164 | if (FD_ISSET(fd, &selectSet))
>>
>> Here's the cc output from ninja:
>>
>> [3/4] cc -Isrc/pgbackrest.p -Isrc -I../../pgbackrest/src
>> -Isrc/command/help -Isrc/postgres -I/usr/include/postgresql
>> -I/usr/include/libxml2 -fdiagnostics-color=always -DNDEBUG
>> -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c99 -O3
>> -Wcast-qual -Wconversion -Wduplicated-cond -Wduplicated-branches
>> -Wformat=2 -Wformat-overflow=2 -Wformat-signedness -Winit-self
>> -Wmissing-prototypes -Wmissing-variable-declarations -Wpointer-arith
>> -Wredundant-decls -Wstrict-prototypes -Wvla -Wwrite-strings
>> -Wno-clobbered -Wno-missing-field-initializers -funroll-loops
>> -ftree-vectorize -fmacro-prefix-map=../../pgbackrest/src/=
>> -fmacro-prefix-map=../../pgbackrest/test/src/=test/
>> -D_POSIX_C_SOURCE=200809L -MD -MQ src/pgbackrest.p/protocol_parallel.c.o
>> -MF src/pgbackrest.p/protocol_parallel.c.o.d -o
>> src/pgbackrest.p/protocol_parallel.c.o -c
>> ../../pgbackrest/src/protocol/parallel.c
>>
>> I can fix this by casting to long unsigned int, but it seems like
>> FD_ISSET() should accept int without complaint like the associate macros
>> and functions.
>>
>> Please CC me on any response since I am not subscribed to the mailing list.
>
> 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.
Regards,
-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.
