svn commit: r211023 - head/usr.sbin/syslogd

[Dag-Erling Smørgrav]

"M. Warner Losh" <imp at bsdimp.com> writes:
> Dag-Erling Smørgrav <des at des.no> writes:
> > "M. Warner Losh" <imp at bsdimp.com> writes:
> > > /*
> > >  * Macros to cast a struct sockaddr, or parts thereof.
> > >  * On architectures with strict alignment requirements, the compiler
> > >  * can bogusly warn about alignment problems since its static analysis
> > >  * is insufficient for it to know that with the APIs used, there
> > >  * really is no alignment issue.
> > >  */
> > That's a bit harsh on the compiler, don't you think?  It never pays to
> > hurt the compiler's feelings :)
>
> /*
>  * Macros to cast a struct sockaddr, or parts thereof.  struct
>  * sockaddr's alginment is loose to later be cast to a sockaddr_in or
>  * sockaddr_in6.  On architectures with strict alignment requirements,
>  * this leads to compiler warnings because the compiler doesn't know
>  * the ABI guarantees proper alignment.
>  */

That sounds more like what I had in mind (my point being that the
compiler is *right* to not make any such assumptions unless we say it's
safe to do so)

> But this leads me to think that the right fix might be:
>
> /*
>  * Structure used by kernel to store most
>  * addresses.
>  */
> struct sockaddr {
> 	unsigned char	sa_len;		/* total length */
> 	sa_family_t	sa_family;	/* address family */
> 	char		sa_data[14];	/* actually longer; address value */
> } __aligned(4);
>
> since that's what the ABI defines....

Yes, unfortunately that's not portable.  I like the way it's done in
sockaddr_storage, but we can't do that here except possibly using
anonymous unions, which aren't portable either.

> > > Why 16 and 4 here?  What's so magical about them?
> > 4 = bytes in a uint32_t, 16 = bytes in an ipv6 address.
> Isn't that better served by 'sizeof(uint32_t)' and
> 'sizeof(ipv6_addr_t)'?

Probably...

DES
-- 
Dag-Erling Smørgrav - des at des.no