Add constructor for creating an AddressList from a struct sockaddr. This is...

net/base/address_list.cc

Index: net/base/address_list.cc
===================================================================
--- net/base/address_list.cc	(revision 76198)
+++ net/base/address_list.cc	(working copy)
@@ -148,6 +148,45 @@
   SetPort(port);
 }
 
+AddressList::AddressList(struct sockaddr* address,
+                         socklen_t address_length,
+                         int socket_type,
+                         int protocol) {
+  // Do sanity checking on socket_type and protocol.
+  DCHECK(socket_type == SOCK_DGRAM || socket_type == SOCK_STREAM);
+  DCHECK(protocol == IPPROTO_TCP || protocol == IPPROTO_UDP);
+
+  struct addrinfo* ai = new addrinfo;
+  memset(ai, 0, sizeof(addrinfo));
+  switch (address_length) {
+    case sizeof(struct sockaddr_in):
+      {
+        struct sockaddr_in* sin =
+            reinterpret_cast<struct sockaddr_in*>(address);
+        ai->ai_family = sin->sin_family;
+        DCHECK_EQ(AF_INET, ai->ai_family);
+      }
+      break;
+    case sizeof(struct sockaddr_in6):
+      {
+        struct sockaddr_in6* sin6 =
+            reinterpret_cast<struct sockaddr_in6*>(address);
+        ai->ai_family = sin6->sin6_family;
+        DCHECK_EQ(AF_INET6, ai->ai_family);
+      }
+      break;
+    default:
+      NOTREACHED() << "Bad IP address";
+      break;
+  }
+  ai->ai_socktype = socket_type;
+  ai->ai_protocol = protocol;
+  ai->ai_addrlen = address_length;
+  ai->ai_addr = reinterpret_cast<struct sockaddr*>(new char[address_length]);
+  memcpy(ai->ai_addr, address, address_length);
+  data_ = new Data(ai, false /*is_system_created*/);
+}
+
 AddressList::AddressList(const AddressList& addresslist)
     : data_(addresslist.data_) {
 }
@@ -219,6 +258,8 @@
 }
 
 const struct addrinfo* AddressList::head() const {
+  if (!data_)
+    return NULL;

[eroman, 2011/03/01 02:52:33]

under what circumstance is this valid?

Seems like we may just be deferring the crash to some other callsite which is
now getting a NULL.

[Mike Belshe, 2011/03/01 19:01:33]

AddressList foo;
if (foo->head())
   blah blah

This would crash.

I find it inconsistent to a) require that we not overload constructors (which
forces things like uninitialized AddressList() objects) and then b) to not
provide graceful accessors around the object (like in this case).  Now we've got
the weird state of both not being able to construct them and not being able to
tell if you've initialized yourself.

[eroman, 2011/03/01 22:11:53]

I disagree.

I think it is wrong for code to be inspecting whether the object is "valid" or
not. It seems like a logic error if consumers are being handed invalid
AddressLists. They should not have to be checking for this (hence crashing IMO
is the appropriate choice).

In fact I would rather we crash when invoking foo->head(), then to be passing
NULL deep into some system library which may not be handling that, and only
crashing later.

   return data_->head;
 }