diff mbox

Fix strict-aliasing warning in resolv/res_hconf.c

Message ID 1432828842.20199.15.camel@ubuntu-sellcey
State Superseded
Headers

Commit Message

Steve Ellcey May 28, 2015, 4 p.m. UTC 
  OK, so here is a complete patch based on Pedro's proposal.  It compiles
fine for me on MIPS with the latest GCC.  OK to checkin?

Steve Ellcey
sellcey@imgtec.com


2015-05-28  Steve Ellcey  <sellcey@imgtec.com>

	* resolv/res_hconf.c (_res_hconf_reorder_addrs): Use a union to
	copy data from cur_ifr->ifr_addr.

Comments

Pedro Alves May 28, 2015, 7:42 p.m. UTC | #1 
On 05/28/2015 05:00 PM, Steve Ellcey wrote:
> OK, so here is a complete patch based on Pedro's proposal.  It compiles
> fine for me on MIPS with the latest GCC.  OK to checkin?

Looks good to me, FWIW.

Thanks,
Pedro Alves
Roland McGrath May 28, 2015, 8:10 p.m. UTC | #2 
Can you verify what effect this has on the compiled code on x86_64?
Steve Ellcey May 28, 2015, 10:09 p.m. UTC | #3 
On Thu, 2015-05-28 at 13:10 -0700, Roland McGrath wrote:
> Can you verify what effect this has on the compiled code on x86_64?

I could, but I would rather not since I normally only build glibc for
MIPS and it always takes me a while to figure out the options and
settings for an x86 build.  I did a build using GCC 4.9.2 for MIPS and
did not see any significant code differences with this patch (i.e. I did
not see an extra data copy on MIPS).

Steve Ellcey
sellcey@imgtec.com
Roland McGrath May 28, 2015, 10:57 p.m. UTC | #4 
> I could, but I would rather not since I normally only build glibc for
> MIPS and it always takes me a while to figure out the options and
> settings for an x86 build.  I did a build using GCC 4.9.2 for MIPS and
> did not see any significant code differences with this patch (i.e. I did
> not see an extra data copy on MIPS).

OK, that's good enough for me to assume that there won't be any extra data
copy on any machine with a reasonable compiler.  (I actually asked about
x86_64 specifically because I wouldn't care about the MIPS code being
suboptimal as long as the x86 code was not.)

The only other thing I'd say about the patch is that the temporary
variable should be declared in the innermost possible scope.


Thanks,
Roland
diff mbox

Patch

diff --git a/resolv/res_hconf.c b/resolv/res_hconf.c
index 73942e8..b9c423e 100644
--- a/resolv/res_hconf.c
+++ b/resolv/res_hconf.c
@@ -410,6 +410,11 @@  _res_hconf_reorder_addrs (struct hostent *hp)
       int sd, num, i;
       /* Save errno.  */
       int save = errno;
+      union
+      {
+	struct sockaddr sa;
+	struct sockaddr_in sin;
+      } ss;
 
       /* Initialize interface table.  */
 
@@ -443,14 +448,14 @@  _res_hconf_reorder_addrs (struct hostent *hp)
 		continue;
 
 	      ifaddrs[new_num_ifs].addrtype = AF_INET;
-	      ifaddrs[new_num_ifs].u.ipv4.addr =
-		((struct sockaddr_in *) &cur_ifr->ifr_addr)->sin_addr.s_addr;
+	      ss.sa = cur_ifr->ifr_addr;
+	      ifaddrs[new_num_ifs].u.ipv4.addr = ss.sin.sin_addr.s_addr;
 
 	      if (__ioctl (sd, SIOCGIFNETMASK, cur_ifr) < 0)
 		continue;
 
-	      ifaddrs[new_num_ifs].u.ipv4.mask =
-		((struct sockaddr_in *) &cur_ifr->ifr_netmask)->sin_addr.s_addr;
+	      ss.sa = cur_ifr->ifr_netmask;
+	      ifaddrs[new_num_ifs].u.ipv4.mask = ss.sin.sin_addr.s_addr;
 
 	      /* Now we're committed to this entry.  */
 	      ++new_num_ifs;