Le samedi 21 janvier 2023, 14:30:11 UTC Alejandro Colomar a écrit :
> Hi Bastien,
>
> On 1/21/23 14:30, Bastien Roucariès wrote:
>
> [...]
>
> >> Ahh, indeed it seems to be UB. It's in the same 6.5.2.3/6: there's a
> >> requirement that the information about the union is kept in the function in
> >> which it's accessed.
> >>
> >> The standard presents an example, which is a bit ambiguous:
> >>
> >> The following is not a valid fragment (because the union type is not
> >> visible within function f):
> >>
>
> [...]
>
> >>
> >> I don't know what's the intention if the union type was global but the
> >> variable
> >> `u` was still not seen by f(). But it seems GCC's interpretation is UB,
> >> according to the test we just saw.
> >>
> >> The solution that I can see for that is making sockaddr also be a union.
> >> That
> >> way, the union is kept across all calls (since they all use sockaddr).
> >>
> >> struct sockaddr {
> >> union {
> >> struct {
> >> sa_family_t sa_family;
> >> char sa_data[14]; // why 14?
> >> }
> >> struct sockaddr_in sin;
> >> struct sockaddr_in6 sin6;
> >> struct sockaddr_un sun;
> >> };
> >> };
> >
> > No the solution is to avoid sockaddr and mark as deprecated.
>
> Declaring `sockaddr` as deprecated means deprecating also:
>
> accept(2)
> accept4(2)
> bind(2)
> connect(2)
> getpeername(2)
> getsockname(2)
> recvfrom(2)
> sendto(2)
> getnameinfo(3)
>
> which use the type in their prototype.
>
> Also, other types such as `addrinfo`, which contain `sockaddr` would also
> need
> to be deprecated, which would itself require deprecating:No because this function will take a opaque transparent union pointer. I mean only raise a warning when user declare a variable (storage) of struct sockaddr... > > getaddrinfo(3) > freeaddrinfo(3) > > Since addrinfo is itself contained in other structures such as `gaicb`, we > would > also need to deprecate those, which would in turn require deprecating more > APIs: > > getaddrinfo_a(3) > gai_error(3) > gai_cancel(3) > > And maybe I left some. This feels like nuking the entire networking API, > which > I don't see happening soon. > > > Otherwise, we need to come up with a solution that keeps these APIs > compatible > with whatever new type we suggest using. Changing them to use `void*` > instead > of `sockaddr*` would be possible, but would decrease type safety > considerably, > so there should be a good reason for that. > > Suggesting to use always `sockaddr_storage` but using `sockaddr` in the > function > parameters keeps the current not-so-nice casting issues, which are not > Undefined > Behavior per se, but not ideal either (in fact, I don't think `void*` is much > worse than code full of casts). And it would also be error-prone, since > users > could get the idea that `sockaddr` can be used safely, since it's what gets > passed as the parameter. > > > The problem it should be part of union without raising a warning each time > > we use a safe type... > > I don't understand this; please detail. the transparent union will include sockaddr, thus even if we use it correctly raise a warning... > > > > > The other solution is to render public and ABI stable the type here > > https://github.com/bminor/glibc/blob/ae612c45efb5e34713859a5facf92368307efb6e/socket/sys/socket.h#L78 > > under for instance sockaddr_ptr and sockaddr_const_ptr > > [[gnu::transparent_union]] (used in that link) seems like a "the design of > this > interface is bad, sorry, this workaround will just make it work". I guess it > just works, and probably it's the reason that so much undefined behavior > hasn't > exploded so far. However, if we can solve this using core language features, > I'd go that way. It solve the problems and could be used without the [[gnu::transparent_union]], c++17 support transparent union. The transparent union should also include pointer to sockaddr_storage and bluetooth socket. Bastien > > > > > Moreover this are some patch arch by arch > > https://sourceware.org/legacy-ml/libc-alpha/2016-02/msg00340.html that > > should be made default > > > > Bastien > > > > > > > >> > >> struct sockaddr_storage { > >> union { > >> sa_family_t ss_family; > >> struct sockaddr sa; > >> }; > >> }; > > Hmm, this isn't still perfect. Since the APIs get the sockaddr, this union > information would be lost. `sockaddr` needs to be the type that is declared. > `sockaddr_storage` should just die; there's no way to make it compatible with > APIs getting a `sockaddr`. The attribute `transparent_union` is the only way > to > use is safely. > > Cheers, > > Alex > > >> > >> > >> With this, sockaddr_storage becomes useless, but still usable. New code, > >> could > >> just use sockaddr and use the internal union members as necessary. Union > >> info > >> is kept across all function boundaries. > > -- > <http://www.alejandro-colomar.es/> >
signature.asc
Description: This is a digitally signed message part.
