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? Doesn't POSIX allow including any other POSIX headers (maybe it does, but IIRC it doesn't)? Since <sys/socket.h> is just a POSIX thing, that's the only standard we should care about.
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:
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]]:
alx@debian:~/tmp$ cat sock.c
#define _GNU_SOURCE
#include <sys/socket.h>
#include <sys/un.h>
int
bind_un(int sfd, const struct sockaddr_un *addr, socklen_t len)
{
return bind(sfd, addr, len);
}
alx@debian:~/tmp$ cc -Wall -Wextra sock.c -S
alx@debian:~/tmp$ clang -Wall -Wextra sock.c -S
alx@debian:~/tmp$
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.
Cheers, Alex
Rich
-- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
