Re: struct sockaddr_storage issues

[Mouse <mouse%Rodents-Montreal.ORG@localhost>]

> The sockaddr_storage structure came after the socket APIs were
> created,

So did almost all of today's computer world. :-)

> My thinking was that connect() really should not care about the exact
> size of the socket structure, as long as it is greater or equal to
> the size required for sa_family_t.

That is not how NetBSD has historically worked, nor, based on your
experience, how it currently works.  While I don't have the historical
code at ready hand to check, I suspect the requirement goes all the way
back to the first released versions of sockets (4.3? 4.2? I forget).

If you think that's broken, well, I disagree, but you can always
send-pr it and let NetBSD decide on its position.

> I try to write portable code for Linux, *BSD, Solaris, etc.

Then you will need to use the correct size value for the address
family.  Anything else will run afoul of systems, like historical
NetBSD and apparently modern NetBSD, that check the size.  I'd be
surprised if Solaris 1 (nee SunOS) didn't also require exact sizes.

> I can't think of a valid reason why connect() should fail for AF_INET
> with larger socket structure, [...]

Are you more interested in writing portable code, or are you more
interested in making NetBSD conform to your idea of `valid'?

I have experience working with socket networking code from BSD 4.3 on
(possibly earlier; I actually used BSD 4.1c briefly, before we got 4.2,
but I don't recall when sockets came in) and I don't think I've
previously seen code that passed a too-large sockaddr length.  (Well,
not for AF_INET or AF_INET6.  Maybe for AF_UNIX, the original name for
today's AF_LOCAL; I seem to recall that early code using it didn't seem
to be clear on what to pass as the address length.)

> Below is fragment of my code that initializes socket address before
> calling connect().  [...]

> struct sockaddr_storage addr;

> /* Set port number */
> port = htons((uint16_t)u64);

> /* Fill sockaddr_storage structure based on sa_family */
> if (sa_family == AF_INET)
> {
> 	((struct sockaddr_in *)&addr)->sin_family = sa_family;
> 	((struct sockaddr_in *)&addr)->sin_port = port;
> 	((struct sockaddr_in *)&addr)->sin_addr = addr_ipv4;
> }
> else if (sa_family == AF_INET6)
> {
> 	((struct sockaddr_in6 *)&addr)->sin6_family = sa_family;
> 	((struct sockaddr_in6 *)&addr)->sin6_port = port;
> 	((struct sockaddr_in6 *)&addr)->sin6_addr = addr_ipv6;
> }

Well, if you want to write portable code, you will need to figure out
the correct size somehow, such as by setting a size variable in the
code blocks where you set up the structs sockaddr.

Two more portability notes:

(1) You likelky will want to set the sin_len and sin6_len fields on
systems that have them.  At a quick glance I see only one place where
NetBSD cares about sin_len fields, that being in the code backing IDENT
lookups, but I think I see a place where it may matter for AF_LOCAL, I
haven't even looked at IPv6, and in any case there may be places I
missed.  It also would surprise me if there weren't systems out there
that cared more.  In your code, depending on addr's storage duration,
you may be getting zero for sin_len and sin6_len or you may be getting
stack trash.

(2) For AF_INET, you also really want to make sure the sockaddr_in is
all zero bits before filling it.  If your addr variable has static
storage duration and you never reuse it, your code is OK in this
respect, and it is a broken requirement, but it _is_ a requirement in
practice for at least some uses, so it likely behooves you to do the
bzero (or equivalent) before filling in the values.  (I once looked at
eliminating that misfeature.  At the time, it would have required
significant kernel-internal overhaul.  When I write code which fills in
structs sockaddr_in, I bzero, though I do comment "XXX API botch".)

/~\ The ASCII				  Mouse
\ / Ribbon Campaign
 X  Against HTML		mouse%rodents-montreal.org@localhost
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B