From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id C6F8823941 for ; Sat, 3 Aug 2024 01:38:16 +0200 (CEST) Received: (qmail 7566 invoked by uid 550); 2 Aug 2024 23:38:09 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 7516 invoked from network); 2 Aug 2024 23:38:08 -0000 Date: Fri, 2 Aug 2024 19:38:01 -0400 From: Rich Felker To: Brad House Cc: musl@lists.openwall.com Message-ID: <20240802233801.GU10433@brightrain.aerifal.cx> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH 1/1] IN6_IS_ADDR_LOOPBACK() and similar macros warn on -Wcast-qual 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) >       |                                                ^ > /__w/c-ares/c-ares/src/lib/ares__sortaddrinfo.c:95:16: warning: cast > from 'const struct in6_addr *' to 'unsigned int *' drops const > qualifier [-Wcast-qual] >    95 |     } else if (IN6_IS_ADDR_LOOPBACK(&addr6->sin6_addr) || >       |                ^ > /usr/include/netinet/in.h:115:24: note: expanded from macro > 'IN6_IS_ADDR_LOOPBACK' >   115 |         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] > == 0 && \ >       |                        ^ > /__w/c-ares/c-ares/src/lib/ares__sortaddrinfo.c:96:16: warning: cast > from 'const struct in6_addr *' to 'unsigned char *' drops const > qualifier [-Wcast-qual] >    96 | IN6_IS_ADDR_LINKLOCAL(&addr6->sin6_addr)) { >       |                ^ > /usr/include/netinet/in.h:123:24: note: expanded from macro > 'IN6_IS_ADDR_LINKLOCAL' >   123 |         ((((uint8_t *) (a))[0]) == 0xfe && (((uint8_t *) > (a))[1] & 0xc0) == 0x80) >       |                        ^ > /__w/c-ares/c-ares/src/lib/ares__sortaddrinfo.c:141:16: warning: > cast from 'const struct in6_addr *' to 'unsigned int *' drops const > qualifier [-Wcast-qual] >   141 |     } else if (IN6_IS_ADDR_V4MAPPED(&addr6->sin6_addr)) { >       |                ^ > /usr/include/netinet/in.h:129:24: note: expanded from macro > 'IN6_IS_ADDR_V4MAPPED' >   129 |         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] > == 0 && \ >       |                        ^ > /__w/c-ares/c-ares/src/lib/ares__sortaddrinfo.c:149:16: warning: > cast from 'const struct in6_addr *' to 'unsigned int *' drops const > qualifier [-Wcast-qual] >   149 |     } else if (IN6_IS_ADDR_V4COMPAT(&addr6->sin6_addr)) { >       |                ^ > /usr/include/netinet/in.h:134:24: note: expanded from macro > 'IN6_IS_ADDR_V4COMPAT' >   134 |         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] > == 0 && \ >       |                        ^ > /__w/c-ares/c-ares/src/lib/ares__sortaddrinfo.c:151:16: warning: > cast from 'const struct in6_addr *' to 'unsigned char *' drops const > qualifier [-Wcast-qual] >   151 |     } else if (IN6_IS_ADDR_SITELOCAL(&addr6->sin6_addr)) { >       |                ^ > /usr/include/netinet/in.h:126:24: note: expanded from macro > 'IN6_IS_ADDR_SITELOCAL' >   126 |         ((((uint8_t *) (a))[0]) == 0xfe && (((uint8_t *) > (a))[1] & 0xc0) == 0xc0) >       |                        ^ > /__w/c-ares/c-ares/src/lib/ares__sortaddrinfo.c:189:16: warning: > cast from 'const struct in6_addr *' to 'unsigned int *' drops const > qualifier [-Wcast-qual] >   189 |     } else if (IN6_IS_ADDR_V4COMPAT(&addr6->sin6_addr) || >       |                ^ > /usr/include/netinet/in.h:134:24: note: expanded from macro > 'IN6_IS_ADDR_V4COMPAT' >   134 |         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] > == 0 && \ >       |                        ^ > > 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_UNSPECIFIED(a) \ > - (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \ > - ((uint32_t *) (a))[2] == 0 && ((uint32_t *) (a))[3] == 0) > + (((const uint32_t *) (a))[0] == 0 && \ > + ((const uint32_t *) (a))[1] == 0 && \ > + ((const uint32_t *) (a))[2] == 0 && \ > + ((const uint32_t *) (a))[3] == 0) > > #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 ) > > -#define IN6_IS_ADDR_MULTICAST(a) (((uint8_t *) (a))[0] == 0xff) > +#define IN6_IS_ADDR_MULTICAST(a) (((const uint8_t *) (a))[0] == 0xff) > > #define IN6_IS_ADDR_LINKLOCAL(a) \ > - ((((uint8_t *) (a))[0]) == 0xfe && (((uint8_t *) (a))[1] & 0xc0) == 0x80) > + ((((const uint8_t *) (a))[0]) == 0xfe && \ > + (((const uint8_t *) (a))[1] & 0xc0) == 0x80) > > #define IN6_IS_ADDR_SITELOCAL(a) \ > - ((((uint8_t *) (a))[0]) == 0xfe && (((uint8_t *) (a))[1] & 0xc0) == 0xc0) > + ((((const uint8_t *) (a))[0]) == 0xfe && \ > + (((const uint8_t *) (a))[1] & 0xc0) == 0xc0) > > #define IN6_IS_ADDR_V4MAPPED(a) \ > - (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \ > - ((uint8_t *) (a))[8] == 0 && ((uint8_t *) (a))[9] == 0 && \ > - ((uint8_t *) (a))[10] == 0xff && ((uint8_t *) (a))[11] == 0xff) > + (((const uint32_t *) (a))[0] == 0 && \ > + ((const uint32_t *) (a))[1] == 0 && \ > + ((const uint8_t *) (a))[8] == 0 && \ > + ((const uint8_t *) (a))[9] == 0 && \ > + ((const uint8_t *) (a))[10] == 0xff && \ > + ((const uint8_t *) (a))[11] == 0xff) > > #define IN6_IS_ADDR_V4COMPAT(a) \ > - (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \ > - ((uint32_t *) (a))[2] == 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))[15] > 1) > > #define IN6_IS_ADDR_MC_NODELOCAL(a) \ > - (IN6_IS_ADDR_MULTICAST(a) && ((((uint8_t *) (a))[1] & 0xf) == 0x1)) > + (IN6_IS_ADDR_MULTICAST(a) && \ > + ((((const uint8_t *) (a))[1] & 0xf) == 0x1)) > > #define IN6_IS_ADDR_MC_LINKLOCAL(a) \ > - (IN6_IS_ADDR_MULTICAST(a) && ((((uint8_t *) (a))[1] & 0xf) == 0x2)) > + (IN6_IS_ADDR_MULTICAST(a) && \ > + ((((const uint8_t *) (a))[1] & 0xf) == 0x2)) > > #define IN6_IS_ADDR_MC_SITELOCAL(a) \ > - (IN6_IS_ADDR_MULTICAST(a) && ((((uint8_t *) (a))[1] & 0xf) == 0x5)) > + (IN6_IS_ADDR_MULTICAST(a) && \ > + ((((const uint8_t *) (a))[1] & 0xf) == 0x5)) > > #define IN6_IS_ADDR_MC_ORGLOCAL(a) \ > - (IN6_IS_ADDR_MULTICAST(a) && ((((uint8_t *) (a))[1] & 0xf) == 0x8)) > + (IN6_IS_ADDR_MULTICAST(a) && \ > + ((((const uint8_t *) (a))[1] & 0xf) == 0x8)) > > #define IN6_IS_ADDR_MC_GLOBAL(a) \ > - (IN6_IS_ADDR_MULTICAST(a) && ((((uint8_t *) (a))[1] & 0xf) == 0xe)) > + (IN6_IS_ADDR_MULTICAST(a) && \ > + ((((const uint8_t *) (a))[1] & 0xf) == 0xe)) > > #define __ARE_4_EQUAL(a,b) \ > (!( (0[a]-0[b]) | (1[a]-1[b]) | (2[a]-2[b]) | (3[a]-3[b]) )) 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