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