Re: struct sockaddr_storage, union (was: Improve getsockname)

["Bastien Roucariès" <rouca@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 3562 bytes --]

Le jeudi 19 janvier 2023, 21:19:49 UTC Alejandro Colomar a écrit :
> 
> On 1/19/23 22:00, Bastien Roucariès wrote:
> [...]
> 
> >> <https://inbox.sourceware.org/libc-alpha/0f25d60f-f183-b518-b6c1-6d46aa63ee57@gmail.com/T/>
> > 
> > I do not believe it is broken by design. It should be used with care and warning.
> > 
> > BTW if we go to the anonymous union way could we add at the end a _null_reserved_field. It will help for unix socket and the infamous sun_path could not end with null...
> > May be it is too late from an ABI point of view, but for me the posix contract from an ABI point of view is that I said in the note  sockaddr_storage  could grow but not be reduced.
> 
> Yes, many types have seen such additions at the end of it over time.  In the 
> Linux man-pages, I try to document all structures as "having at least these 
> members", but may grow over time.

In fact it is not needed and it is the best argument of struct sockaddr_storage
 printf("%li %li",sizeof(struct sockaddr_storage),sizeof(struct sockaddr_un));
give me 128 vs 110...

So if correctly documented and aliasing solved it will be the best of the world...

Moreover kernel expect it https://elixir.bootlin.com/linux/latest/source/net/unix/af_unix.c#L293

> 
> > 
> >   struct sockaddr_storage {
> > 	union {
> >   		sa_family_t          ss_family;
> >   		struct sockaddr      sa;
> >   		struct sockaddr_in   sin;
> >   		struct sockaddr_in6  sin6;
> >   		struct sockaddr_un   sun;
> >   	};
> >                         char __reserved_null;
> 
> Such a field would make sense.  In fact, I believe the Linux internal 
> implementation of _un must have something similar, since it ensures 
> null-termination even if the user passes a non-terminated string, IIRC.
> 
> >   };
> > 
> [...]
> 
> >> This is compatible:
> >>
> >> -  It had at least the `ss_family` field.  It's still there, at the same binary
> >> location.
> >> -  It has a size at least as large as any other sockaddr_* structure, and a
> >> suitable alignment.
> >> -  Old code still works with it just fine.
> >> -  New code will be able to avoid UB, and all casts, just by accessing the right
> >> structure element.
> >> -  It's trivial to test at configure time if the implementation provides this
> >> new definition of the structure.
> > 
> > I agree I could even add a macro for autoconf-archive (I am upstream) and post a patch for gnulib.
> 
> Nice; since it's backwards compatible, I'll (probably) suggest a patch for glibc.
> 
> > 
> >>>>
> >>>>> +.I sockaddr_storage
> >>>>> +structure is large enough to hold any of the other
> >>>>> +.I sockaddr_*
> >>>>> +variants and always well aligned. On return, it should be cast to the correct
> >>>>> +.I sockaddr_*
> >>>>
> >>>> The fact that it is correctly aligned, and a cast will work most of the time,
> >>>> isn't enough for strict aliasing rules.  The compiler is free to assume things,
> >>>> just by the fact that it's a different type.
> >>>
> >>> Ok any idea for writing this kind of stuff
> >>
> >> I'm thinking about writing something to several pages; will keep you all updated
> >> on important changes to the pages.
> > 
> > Please exchange with me... It is really a pitffall for my student, so I could help here.
> 
> Sure.  Will do.
> 
> Cheers,
> 
> Alex
> 
> > 
> > Bastien
> >>
> >>
> >> Cheers,
> >>
> >> Alex
> >>
> >> -- 
> >> <http://www.alejandro-colomar.es/>
> >>
> > 
> 
> -- 
> <http://www.alejandro-colomar.es/>
> 


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]