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

net/base/address_list.h

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_BASE_ADDRESS_LIST_H_
 #define NET_BASE_ADDRESS_LIST_H_
 #pragma once
 
 #include <string>
 
 #include "base/ref_counted.h"
 #include "net/base/net_util.h"
 
 struct addrinfo;
 
 namespace net {
 
 // An AddressList object contains a linked list of addrinfo structures.  This
 // class is designed to be copied around by value.
 class AddressList {
  public:
   // Constructs an empty address list.
   AddressList();
 
   // Constructs 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);
 
+  // Constructs an address list for a single socket address.
+  // |socket_type| is either SOCK_STREAM or SOCK_DGRAM
+  // |protocol| is either IPPROTO_TCP or IPPROTO_UDP
+  AddressList(struct sockaddr* address, socklen_t address_length,

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

nit: I have a personal hatred towards constructor overloading.

Could you make this a named static method, like say:

static AddressList CreateFromSockaddr(...);


(I would recommend the same for the constructor call above so it is less
special.... CreateAddressListFromNumber(...), I don't expect you to do that in
this CL tho.)

[eroman, 2011/03/01 02:53:09]

Please make it a |const struct sockaddr*| to emphasize it does not take
ownership.

Also please mention in the comment that it 

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

I don't understand why - especially for a case like this.

I could see that objection, especially for cases where you end up with redundant
code in the constructor.  But in this case, that isn't true.  Further, I now
lose the ability to create these on the stack, and have to use the heap.  Yuck.

I'll make the change, but I find it arbitrary.

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

Done.

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

The problem I have with constructor overloading, is can hurt readability of
code. When you have just 1 or 2 constructors it is manageable, and I would be OK
with leaving this as a constructor. However once more variants start to get
added, it gets icky.

At the extreme, take the MockRead class, which zillions of constructors. I find
it hurts the readability of that code, since constructors are essentially
unnamed functions (i can never tell what the heck those ctors are doing just by
reading the callsite). Whereas static factory methods solve this by giving named
actions to the creation.

As far as the performance impact, I think that is a non-issue. Ultimately you
are still heap-allocating the "Data" member either way, which will dominate the
cost. (I claim the extra cost of having made a stack copy is negligeable). In
fact in practice when we compile with visual studio's WPO, it is able to
recognize this pattern and will avoid the function-local copy.

Anyhow, I am voicing my preference here, I don't want to fight over these nits
:) Whatever you choose to do is fine for this CL.

+              int socket_type, int protocol);
+
   AddressList(const AddressList& addresslist);
   ~AddressList();
   AddressList& operator=(const AddressList& addresslist);
 
   // Adopt 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);
 
@@ skipping 38 matching lines @@
   struct Data;
 
   explicit AddressList(Data* data);
 
   scoped_refptr<Data> data_;
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_ADDRESS_LIST_H_