[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4c03697d-36fd-42e3-8a27-e121d3acc522@brad-house.com>
Date: Fri, 2 Aug 2024 20:02:16 -0400
From: Brad House <brad@...d-house.com>
To: musl@...ts.openwall.com
Cc: Rich Felker <dalias@...c.org>
Subject: Re: [PATCH 1/1] IN6_IS_ADDR_LOOPBACK() and similar macros warn
on -Wcast-qual
On 8/2/24 7:38 PM, Rich Felker wrote:
> On Fri, Aug 02, 2024 at 05:27:26PM -0400, Brad House wrote:
>> I'm the maintainer of c-ares (https://c-ares.org) and have been
>> scanning the CI build logs for various systems to catch warnings,
>> and on Alpine Linux (which obviously uses musl c) we get these
>> warnings, specifically when using clang (but not oddly not on gcc):
>>
>> /__w/c-ares/c-ares/src/lib/ares__sortaddrinfo.c:93:9: warning: cast
>> from 'const struct in6_addr *' to 'unsigned char *' drops const
>> qualifier [-Wcast-qual]
>> 93 | if (IN6_IS_ADDR_MULTICAST(&addr6->sin6_addr)) {
>> | ^
>> /usr/include/netinet/in.h:120:48: note: expanded from macro
>> 'IN6_IS_ADDR_MULTICAST'
>> 120 | #define IN6_IS_ADDR_MULTICAST(a) (((uint8_t *) (a))[0] == 0xff)
>> | ^
>> ... ^
>>
>> Full build output: https://github.com/c-ares/c-ares/actions/runs/10219723015/job/28278549865
>>
>> I've attached a patch that will silence this warning by always
>> casting to the comparison to const, but otherwise not impact the
>> behavior.
>>
>> -Brad
>> diff --git a/include/netinet/in.h b/include/netinet/in.h
>> index fb628b61..f04657f3 100644
>> --- a/include/netinet/in.h
>> +++ b/include/netinet/in.h
>> @@ -108,46 +108,63 @@ uint16_t ntohs(uint16_t);
>> #define IPPROTO_MAX 263
>>
>> ...
>> #define IN6_IS_ADDR_LOOPBACK(a) \
>> - (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \
>> - ((uint32_t *) (a))[2] == 0 && \
>> - ((uint8_t *) (a))[12] == 0 && ((uint8_t *) (a))[13] == 0 && \
>> - ((uint8_t *) (a))[14] == 0 && ((uint8_t *) (a))[15] == 1 )
>> + (((const uint32_t *) (a))[0] == 0 && \
>> + ((const uint32_t *) (a))[1] == 0 && \
>> + ((const uint32_t *) (a))[2] == 0 && \
>> + ((const uint8_t *) (a))[12] == 0 && \
>> + ((const uint8_t *) (a))[13] == 0 && \
>> + ((const uint8_t *) (a))[14] == 0 && \
>> + ((const uint8_t *) (a))[15] == 1 )
>> ...
> It looks like there's a lot wrong with these macros. They should not
> be doing random pointer casts like they are. Per the standard, they
> take an argument of type const struct in6_addr *, so they should
> almost surely be operating on that type directly. That would make them
> actually type-safe (diagnostic if called with wrong argument type).
>
> I guess we should look at whether there's any good reason they were
> written the way they were..
>
> Rich
Yep, I see what you mean. There are already accessors for 8, 16, and
32bit into struct in6_addr so its odd not to use those. I've attached a
v2 patch that uses those instead which also cleans up the warnings.
-Brad
View attachment "musl-netinet-warning-fix-v2.patch" of type "text/plain" (3845 bytes)
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.
