From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from brightrain.aerifal.cx (brightrain.aerifal.cx [216.12.86.13]) by sourceware.org (Postfix) with ESMTPS id 4E4133858C83 for ; Mon, 6 Feb 2023 00:15:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4E4133858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=libc.org Authentication-Results: sourceware.org; spf=none smtp.mailfrom=libc.org Date: Sun, 5 Feb 2023 19:15:47 -0500 From: Rich Felker To: Alejandro Colomar Cc: linux-man@vger.kernel.org, Alejandro Colomar , GCC , glibc , Bastien =?utf-8?Q?Roucari=C3=A8s?= , Stefan Puiu , Igor Sysoev , Andrew Clayton , Richard Biener , Zack Weinberg , Florian Weimer , Joseph Myers , Jakub Jelinek , Eric Blake Subject: Re: [PATCH] sockaddr.3type: BUGS: Document that libc should be fixed using a union Message-ID: <20230206001546.GG3298@brightrain.aerifal.cx> References: <20230205152835.17413-1-alx@kernel.org> <20230205234359.GF3298@brightrain.aerifal.cx> <29bb163d-ab7b-2fb7-a1c8-cfd720984a8d@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <29bb163d-ab7b-2fb7-a1c8-cfd720984a8d@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Feb 06, 2023 at 12:59:48AM +0100, Alejandro Colomar wrote: > Hi Rich, > > On 2/6/23 00:43, Rich Felker wrote: > >On Sun, Feb 05, 2023 at 04:28:36PM +0100, Alejandro Colomar wrote: > >>As discussed before, and Bastien and I seem to agree, ideally we should > >>define the following types: > >> > >> struct sockaddr_storage { > >> union { > >> struct { > >> sa_family_t ss_family; > >> }; > >> struct sockaddr_in sin; > >> struct sockaddr_in6 sin6; > >> struct sockaddr_un sun; > >> // ... > >> }; > >> }; > > > >AFAIK this is not permitted because of namespace. sys/socket.h is not > >permitted to expose sockaddr_{in,in6,un}. And if you defined > >differently-tagged structures with the same contents, it would not do > >any good; accessing the members with the wrong-tagged struct type > >would still be UB. > > I'm not sure. ISO C has that restriction that a header can only > define what the standard says it defines. However, does POSIX have > the same restriction? Yes. > Doesn't POSIX allow including any other POSIX > headers (maybe it does, but IIRC it doesn't)? Not except where it's explicitly allowed. > Since > is just a POSIX thing, that's the only standard we should care > about. The relevant text is here: https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_02_02 > >Really, there is no action needed here. Nothing is wrong on libc's > >side. The problem is just that the type is *not useful for anything* > >and should not be used except in the context of sizeof, which is > >purely a documentation issue. > > > >> struct [[deprecated]] sockaddr { > >> sa_family_t sa_family; > >> }; > >> > >> union [[gnu::transparent_union]] sockaddr_ptr { > >> struct sockaddr_storage *ss; > >> struct sockaddr *sa; > >> }; > >> > >>And then we could define APIs like: > >> > >> int bind(int sockfd, const union sockaddr_ptr *addr, socklen_t len); > > > >You cannot just change APIs because you wish they were different. > > This API is compatible. In fact, it already is now like that: Unless I'm mistaken, it's not. The function pointer the name 'bind' produces will not have the right type, and will have problems being stored in a function pointer object with the right type, as well as wrong results with _Generic, etc. Only plain calls to it are unaffected. > alx@debian:/usr/include$ grepc bind > ./x86_64-linux-gnu/sys/socket.h:112: > extern int bind (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len) > __THROW; > > alx@debian:/usr/include$ sed -n 83,84p x86_64-linux-gnu/sys/socket.h > typedef union { __SOCKADDR_ALLTYPES > } __CONST_SOCKADDR_ARG __attribute__ ((__transparent_union__)); > > > >Ideally bind, etc. would just take void *, > > void * is a bit too much unsafe. GCC's transparent unions are a > restricted catch-many pointer, rather than a catch-all. > > >which is what the struct > >sockaddr * is being used as. > > And in fact, void* wouldn't sole the union problem. > > >But they don't, so callers have to cast. > > With the current glibc implementation, you don't need to cast, > thanks to the [[gnu::transparent_union]]: This is just facilitating writing non-portable code, since correct code written to the spec *does* have to cast. > >It's ugly but it's really not a big deal. Much less of a big deal than > >breaking the interface because you think it would look prettier if it > >had been done differently. > > It's not breaking the interface; not in GNU C. Current code still > falls back to the a POSIX-complying UB-invoking interface when you > don't ask for _GNU_SOURCE, but we can keep that. I'm only asking > that we fix the GNU C version. Moreover, in POSIX-complying code, > you can keep the interface pointer, since you'll need to cast > anyway, but can still make sockaddr_storage be implemented through > an anonymous union. If it's only for _GNU_SOURCE it's probably mostly harmless, and allowable to violate the namespace rules, but also not terribly helpful... Rich