Miscelaneous cleanups to AddressList to make it harder to mis-use.

 - Removed all destructive non-const member functions -- these were dangerous since if you called them without first making a copy of the AddressList, it could mutate earlier copies.

 - Made AddressList::Data::head const, so new code added to AddressList cannot inadvertently introduce such dangerous mutations (won't compile).

 - Moved the non-trivial constructors and assign methods into factory methods (for added readability)

 - Removed the bool parameter from Copy (for added readability).

Review URL: http://codereview.chromium.org/6880302

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85090 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 40 insertions, 25 deletions

diff --git a/net/base/address_list.h b/net/base/address_list.h
index 0efd77f..1ac4e0e 100644
--- a/net/base/address_list.h
+++ b/net/base/address_list.h
@@ -11,6 +11,8 @@
 #include "base/memory/ref_counted.h"
 #include "net/base/net_util.h"
 
+// TODO(eroman): Fix the declaration + definition ordering to match style guide.
+
 struct addrinfo;
 
 namespace net {
@@ -19,46 +21,55 @@ namespace net {
 // class is designed to be copied around by value.
 class AddressList {
  public:
-  // Constructs an empty address list.
+  // Constructs an invalid address list. Should not call any methods on this
+  // other than assignment.
   AddressList();
 
-  // Constructs an address list for a single IP literal.  If
+  // Creates an address list for a single IP literal.
+  static AddressList CreateFromIPAddress(
+      const IPAddressNumber& address,
+      uint16 port);
+
+  // Creates an address list for a single IP literal.  If
   // |canonicalize_name| is true, fill the ai_canonname field with the
   // canonicalized IP address.
-  AddressList(const IPAddressNumber& address, int port, bool canonicalize_name);
+  static AddressList CreateFromIPAddressWithCname(
+      const IPAddressNumber& address,
+      uint16 port,
+      bool canonicalize_name);
 
   AddressList(const AddressList& addresslist);
   ~AddressList();
   AddressList& operator=(const AddressList& addresslist);
 
-  // Adopt the given addrinfo list (assumed to have been created by
+  // Adopts the given addrinfo list (assumed to have been created by
   // the system, e.g. returned by getaddrinfo()) in place of the
   // existing one if any.  This hands over responsibility for freeing
   // the addrinfo list to the AddressList object.
-  void Adopt(struct addrinfo* head);
+  static AddressList CreateByAdoptingFromSystem(struct addrinfo* head);
+
+  // Creates a new address list with a copy of |head|. This includes the
+  // entire linked list.
+  static AddressList CreateByCopying(const struct addrinfo* head);
 
-  // Copies the given addrinfo rather than adopting it. If |recursive| is true,
-  // all linked struct addrinfos will be copied as well. Otherwise only the head
-  // will be copied, and the rest of linked entries will be ignored.
-  void Copy(const struct addrinfo* head, bool recursive);
+  // Creates a new address list wich has a single address, |head|. If there
+  // are other addresses in |head| they will be ignored.
+  static AddressList CreateByCopyingFirstAddress(const struct addrinfo* head);
 
   // Appends a copy of |head| and all its linked addrinfos to the stored
-  // addrinfo.
+  // addrinfo. Note that this will cause a reallocation of the linked list,
+  // which invalidates the head pointer.
   void Append(const struct addrinfo* head);
 
   // Sets the port of all addresses in the list to |port| (that is the
-  // sin[6]_port field for the sockaddrs).
-  void SetPort(int port);
+  // sin[6]_port field for the sockaddrs). Note that this will cause a
+  // reallocation of the linked list, which invalidates the head pointer.
+  void SetPort(uint16 port);
 
   // Retrieves the port number of the first sockaddr in the list. (If SetPort()
   // was previously used on this list, then all the addresses will have this
   // same port number.)
-  int GetPort() const;
-
-  // Sets the address to match |src|, and have each sockaddr's port be |port|.
-  // If |src| already has the desired port this operation is cheap (just adds
-  // a reference to |src|'s data.) Otherwise we will make a copy.
-  void SetFrom(const AddressList& src, int port);
+  uint16 GetPort() const;
 
   // Gets the canonical name for the address.
   // If the canonical name exists, |*canonical_name| is filled in with the
@@ -67,18 +78,22 @@ class AddressList {
   // |canonical_name| must be a non-null value.
   bool GetCanonicalName(std::string* canonical_name) const;
 
-  // Clears all data from this address list. This leaves the list in the same
-  // empty state as when first constructed.
-  void Reset();
-
-  // Get access to the head of the addrinfo list.
+  // Gets access to the head of the addrinfo list.
+  //
+  // IMPORTANT: Callers SHOULD NOT mutate the addrinfo chain, since under the
+  //            hood this data might be shared by other AddressLists, which
+  //            might even be running on other threads.
+  //
+  //            Additionally, non-const methods like SetPort() and Append() can
+  //            cause the head to be reallocated, so do not cache the return
+  //            value of head() across such calls.
   const struct addrinfo* head() const;
 
-  // Constructs an address list for a single socket address.
+  // Creates an address list for a single socket address.
   // |address| the sockaddr to copy.
   // |socket_type| is either SOCK_STREAM or SOCK_DGRAM.
   // |protocol| is either IPPROTO_TCP or IPPROTO_UDP.
-  static AddressList* CreateAddressListFromSockaddr(
+  static AddressList CreateFromSockaddr(
       const struct sockaddr* address,
       socklen_t address_length,
       int socket_type,