Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Aug 2010 21:56:05 +0200 (CEST)
From:      Oliver Fromme <olli@fromme.com>
To:        imp@bsdimp.com (M. Warner Losh)
Cc:        src-committers@FreeBSD.org, jilles@stack.nl, svn-src-all@FreeBSD.org, olli@FreeBSD.org, svn-src-head@FreeBSD.org, des@des.no
Subject:   Re: svn commit: r211023 - head/usr.sbin/syslogd
Message-ID:  <201008101956.o7AJu5ms044774@haluter.fromme.com>
In-Reply-To: <20100810.120103.69891821625677670.imp@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help

M. Warner Losh wrote:
 > /*
 >  * 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.
 >  */
 > 
 > 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, that would solve most of the problems, at least the ones
related to struct sockaddr.

Can we make that change to struct sockaddr, or does it cause
unwanted side-effects?

I could do a full "make universe" to test it, but it would
probably take two days on my 9-current machine.

 > : > : @@ -2410,8 +2419,8 @@
 > : > :  				}
 > : > :  				reject = 0;
 > : > :  				for (j = 0; j < 16; j += 4) {
 > : > : -					if ((*(u_int32_t *)&sin6->sin6_addr.s6_addr[j] & *(u_int32_t *)&m6p->sin6_addr.s6_addr[j])
 > : > : -					    != *(u_int32_t *)&a6p->sin6_addr.s6_addr[j]) {
 > : > : +					if ((UINT32_CAST(sin6->sin6_addr.s6_addr[j]) & UINT32_CAST(m6p->sin6_addr.s6_addr[j]))
 > : > : +					    != UINT32_CAST(a6p->sin6_addr.s6_addr[j])) {
 > : > :  						++reject;
 > : > :  						break;
 > : > :  					}
 > : > : 
 > : > : 
 > : >
 > : > 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)'?

Yes, probably.

Best regards
   Oliver

-- 
``We are all but compressed light'' (Albert Einstein)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008101956.o7AJu5ms044774>