Re: [PATCH] rpcbind: add support for systemd socket activation

[Lennart Poettering <lennart@poettering.net>]

On Wed, 01.02.12 17:03, Chuck Lever (chuck.lever@oracle.com) wrote:

> >>> Why is sockaddr_storage included in this union?
> >> 
> >> This is from Lennart's original patch. Lennart, care to comment?
> > 
> > Well, simply because sockaddr_storage is the actual struct one should
> > normally use for this kind of thing (where you try to determine a
> > sockaddr from a socket you don't know at all). With one exception
> > however, sockaddr_un is actually longer than sockaddr_storage, which is
> > documented borkedness in the socket API.
> 
> You don't reference any of the fields inside this union, except for sa.  It seems unnecessary to include all of these members, and then not use most of them.
> 
> The biggest address you're ever going to get out of libtirpc is a sockaddr_storage.  If we must stick with a union to prevent pointer aliasing, can we have just two members: sockaddr and sockaddr_storage?
> 
> Otherwise, there's no need for this kind of generality here:  TI-RPC handles only IPv4, IPv6, and AF_LOCAL.

Well, as said, sockaddr_un is actually larger than sockaddr_storage, and
hence you definitely do need sockaddr_un in the union. The IP cases are
covered by sockaddr_storage however, and hence can be left out of the
union.

I mean, it's completely up to you what is used here, I just figured the
point of the whole library was to be an exercise in keeping things
independent from the used socket family.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.