Cleanup GetNetworkList.

net/base/net_util_posix.cc

Index: net/base/net_util_posix.cc
diff --git a/net/base/net_util_posix.cc b/net/base/net_util_posix.cc
index 9257989b9f6d76e7c2797be8f9598a6e887350c3..29c93ca4d7242b27f0b095544e0be79824e1e904 100644
--- a/net/base/net_util_posix.cc
+++ b/net/base/net_util_posix.cc
@@ -19,6 +19,8 @@
 #if !defined(OS_ANDROID)
 #include <ifaddrs.h>
 #endif
+#include <net/if.h>
+#include <netinet/in.h>
 
 namespace net {
 
@@ -68,28 +70,50 @@ bool GetNetworkList(NetworkInterfaceList* networks) {
   // getifaddrs() may require IO operations.
   base::ThreadRestrictions::AssertIOAllowed();
 
-  ifaddrs *ifaddr;
-  if (getifaddrs(&ifaddr) < 0) {
+  ifaddrs *interfaces;
+  if (getifaddrs(&interfaces) < 0) {
     PLOG(ERROR) << "getifaddrs";
     return false;
   }
 
-  for (ifaddrs *ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) {
-    if (!ifa->ifa_addr)
-      continue; // Interface has no IP addresses, so skip it.
-    int family = ifa->ifa_addr->sa_family;
-    if (family == AF_INET || family == AF_INET6) {
-      IPEndPoint address;
-      std::string name = ifa->ifa_name;
-      if (address.FromSockAddr(ifa->ifa_addr,
-                               sizeof(ifa->ifa_addr)) &&
-          name.substr(0, 2) != "lo") {
-        networks->push_back(NetworkInterface(name, address.address()));
-      }
+  // Enumerate the addresses assigned to network interfaces which are up.
+  for (ifaddrs *interface = interfaces;
+       interface != NULL;
+       interface = interface->ifa_next) {
+    // Skip loopback interfaces, and ones which are down.
+    if (!(IFF_UP & interface->ifa_flags))
+      continue;
+    if (IFF_LOOPBACK & interface->ifa_flags)
+      continue;
+    // Skip interfaces with no address configured.
+    struct sockaddr* addr = interface->ifa_addr;
+    if (!addr)
+      continue;
+    // Skip loopback addresses configured on non-loopback interfaces.
+    if (addr->sa_family == AF_INET6) {
+      struct sockaddr_in6* addr_in6 =
+          reinterpret_cast<struct sockaddr_in6*>(addr);
+      struct in6_addr* sin6_addr = &addr_in6->sin6_addr;
+      if (IN6_IS_ADDR_LOOPBACK(sin6_addr))
+        continue;
+    } else if (addr->sa_family == AF_INET) {
+      struct sockaddr_in* addr_in =
+          reinterpret_cast<struct sockaddr_in*>(addr);
+      if (addr_in->sin_addr.s_addr == INADDR_LOOPBACK)
+        continue;
+    } else {
+      // Skip non-IP addresses.
+      continue;
+    }
+    IPEndPoint address;
+    std::string name = interface->ifa_name;
+    if (address.FromSockAddr(interface->ifa_addr,
+                             sizeof(interface->ifa_addr))) {

[wtc, 2011/08/15 22:38:31]

Change interface->ifa_addr (two occurrences) to your local
variable 'addr'.

BUG: the second argument to address.FromSockAddr() is wrong.
It should be the size of the particular kind of sockaddr
structure.  So it should be
  sizeof(struct sockaddr_in6)
or
  sizeof(struct sockaddr_in)
depending on the value of addr->sa_family.

What you're passing here, sizeof(interface->ifa_addr),
is the size of a pointer.

The reason it works is that address.FromSockAddr()
ignores the second argument.  Sigh.

I think you should pass a value that's more "correct",
such as sizeof(*addr) (note the '*'), or a value that's
clearly wrong, such as 0.  I would use a local variable
size_t address_length that's set in the if-else-if statement
on lines 93-103 above.

+      networks->push_back(NetworkInterface(name, address.address()));
     }
   }
 
-  freeifaddrs(ifaddr);
+  freeifaddrs(interfaces);
 
   return true;
 #endif