[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0c1ddb63-8bb9-4bfc-918d-3fa4f55fcfa4@brad-house.com>
Date: Thu, 18 Jul 2024 11:21:59 -0400
From: Brad House <brad@...d-house.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/1] FD_SET() and FD_ISSET() warn on
-Wsign-conversion
On 7/18/24 9:19 AM, Szabolcs Nagy wrote:
> the macro argument must be protected with ()
> e.g. the patch changes behaviour if d is 0.5?x:y
>
> and musl prefers 'unsigned' to 'unsigned int', so
>
> #define FD_SET(d, s) ((s)->fds_bits[(unsigned)(d)/(8*sizeof(long))] |= (1UL<<((unsigned)(d)%(8*sizeof(long)))))
>
> note that this change can hide real bugs: now
> int (d) is implicit converted to size_t and e.g
> pointer types error out, but the cast suppresses
> the error for pointers, not just the signedness
> warning.
>
> i think only cast can suppress the warning (at
> least if we stick to standard c) and suspect
> this is why the warning is not used widely: it
> forces changes that make the code more buggy.
>
> we used to argue that compilers should not warn
> about system headers even if it's macro expansion
> but when it's partly system header and partly
> user code (d) then compilers tend to warn anyway.
> i think for your project the cleanest is to wrap
> FD_* and then you can do whatever.
I'm not sure about your statement above about a behavior change if d is
0.5, file descriptors are required to be int, which can't represent
0.5. Passing in a garbage value would clearly fall into "Undefined
Behavior".
We really can't wrap FD_SET/FD_ISSET in c-ares, since wrapping it would
require the library to know the internals about the operations of the
fd_set for each and every platform, which could change over time if
there are ABI changes.
I understand your concern regarding casting away real programming errors
by accident. The only solution to that I see would be to turn the
macros into functions, POSIX says either are valid:
https://pubs.opengroup.org/onlinepubs/009604499/basedefs/sys/select.h.html
Of course, that would introduce a backwards-incompatible ABI change
where an application compiled against a newer version of musl wouldn't
run on an older version. I don't know what your ABI policies look like
to know if that's acceptable.
-Brad
Content of type "text/html" skipped
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.
