diff mbox

Fix strict-aliasing warning in resolv/res_hconf.c

Message ID 1432855124.20199.35.camel@ubuntu-sellcey
State Committed
Headers

Commit Message

Steve Ellcey May 28, 2015, 11:18 p.m. UTC 
  On Thu, 2015-05-28 at 15:57 -0700, Roland McGrath wrote:
> > 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

OK, moving the declaration is easy enough to do, I will put it in the
for loop where it is used.

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

Roland McGrath May 28, 2015, 11:43 p.m. UTC | #1 
> 	* resolv/res_hconf.c (_res_hconf_reorder_addrs): Use a union to
> 	copy data from cur_ifr->ifr_addr.
                       CUR_IFR->ifr_addr and CUR_IFR->ifr_netmask.

OK to commit with the log fix.


Thanks,
Roland
Steve Ellcey May 29, 2015, 5:53 p.m. UTC | #2 
On Thu, 2015-05-28 at 16:43 -0700, Roland McGrath wrote:
> > 	* resolv/res_hconf.c (_res_hconf_reorder_addrs): Use a union to
> > 	copy data from cur_ifr->ifr_addr.
>                        CUR_IFR->ifr_addr and CUR_IFR->ifr_netmask.
> 
> OK to commit with the log fix.
> 
> 
> Thanks,
> Roland

Is there a reason you want CUR_IFR in upper case?  It is not that way in
the code.

Steve Ellcey
sellcey@imgtec.com
Roland McGrath June 5, 2015, 8:51 p.m. UTC | #3 
> Is there a reason you want CUR_IFR in upper case?  It is not that way in
> the code.

The convention for comments and log entries is to put local variable and
parameter names in upper case.
diff mbox

Patch

diff --git a/resolv/res_hconf.c b/resolv/res_hconf.c
index 73942e8..b9c229d 100644
--- a/resolv/res_hconf.c
+++ b/resolv/res_hconf.c
@@ -439,18 +439,24 @@  _res_hconf_reorder_addrs (struct hostent *hp)
 	  for (cur_ifr = ifr, i = 0; i < num;
 	       cur_ifr = __if_nextreq (cur_ifr), ++i)
 	    {
+	      union
+	      {
+		struct sockaddr sa;
+		struct sockaddr_in sin;
+	      } ss;
+
 	      if (cur_ifr->ifr_addr.sa_family != AF_INET)
 		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;