Reimplements net::AddressList without struct addrinfo.

net/socket/tcp_client_socket_win.cc

 // Copyright (c) 2012 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.
 
 #include "net/socket/tcp_client_socket_win.h"
 
 #include <mstcpip.h>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/metrics/stats_counters.h"
 #include "base/string_util.h"
 #include "base/win/object_watcher.h"
 #include "base/win/windows_version.h"
 #include "net/base/address_list_net_log_param.h"
 #include "net/base/connection_type_histograms.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/base/network_change_notifier.h"
-#include "net/base/sys_addrinfo.h"
 #include "net/base/winsock_init.h"
 #include "net/base/winsock_util.h"
 
 namespace net {
 
 namespace {
 
 bool SetSocketReceiveBufferSize(SOCKET socket, int32 size) {
   int rv = setsockopt(socket, SOL_SOCKET, SO_RCVBUF,
                       reinterpret_cast<const char*>(&size), sizeof(size));
@@ skipping 276 matching lines @@
 }
 
 //-----------------------------------------------------------------------------
 
 TCPClientSocketWin::TCPClientSocketWin(const AddressList& addresses,
                                        net::NetLog* net_log,
                                        const net::NetLog::Source& source)
     : socket_(INVALID_SOCKET),
       bound_socket_(INVALID_SOCKET),
       addresses_(addresses),
-      current_ai_(NULL),
+      current_address_index_(static_cast<size_t>(-1)),
       waiting_read_(false),
       waiting_write_(false),
       next_connect_state_(CONNECT_STATE_NONE),
       connect_os_error_(0),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SOCKET)),
       previously_disconnected_(false),
       num_bytes_read_(0) {
   scoped_refptr<NetLog::EventParameters> params;
   if (source.is_valid())
     params = new NetLogSourceParameter("source_dependency", source);
@@ skipping 11 matching lines @@
 
   int error = SetupSocket(socket);
   if (error)
     return MapSystemError(error);
 
   socket_ = socket;
   SetNonBlocking(socket_);
 
   core_ = new Core(this);
 
-  current_ai_ = addresses_.head();
+  current_address_index_ = 0;
   use_history_.set_was_ever_connected();
 
   return OK;
 }
 
 int TCPClientSocketWin::Bind(const IPEndPoint& address) {
-  if (current_ai_ != NULL || bind_address_.get()) {
+  if (current_address_index_ < addresses_.size() || bind_address_.get()) {
     // Cannot bind the socket if we are already connected or connecting.
     return ERR_UNEXPECTED;
   }
 
-  sockaddr_storage addr_storage;
-  sockaddr* addr = reinterpret_cast<struct sockaddr*>(&addr_storage);
-  size_t addr_len = sizeof(addr_storage);
-  if (!address.ToSockAddr(addr, &addr_len))
+  SockaddrStorage storage;
+  if (!address.ToSockAddr(storage.addr, &storage.addr_len))
     return ERR_INVALID_ARGUMENT;
 
   // Create |bound_socket_| and try to bound it to |address|.
   int error = CreateSocket(address.GetFamily(), &bound_socket_);
   if (error)
     return MapSystemError(error);
 
-  if (bind(bound_socket_, addr, addr_len)) {
+  if (bind(bound_socket_, storage.addr, storage.addr_len)) {
     error = errno;
     if (closesocket(bound_socket_) < 0)
       PLOG(ERROR) << "closesocket";
     bound_socket_ = INVALID_SOCKET;
     return MapSystemError(error);
   }
 
   bind_address_.reset(new IPEndPoint(address));
 
   return 0;
 }
 
 
 int TCPClientSocketWin::Connect(const CompletionCallback& callback) {
   DCHECK(CalledOnValidThread());
 
   // If already connected, then just return OK.
   if (socket_ != INVALID_SOCKET)
     return OK;
 
   base::StatsCounter connects("tcp.connect");
   connects.Increment();
 
   net_log_.BeginEvent(NetLog::TYPE_TCP_CONNECT,
                       new AddressListNetLogParam(addresses_));
 
   // We will try to connect to each address in addresses_. Start with the
   // first one in the list.
   next_connect_state_ = CONNECT_STATE_CONNECT;
-  current_ai_ = addresses_.head();
+  current_address_index_ = 0;
 
   int rv = DoConnectLoop(OK);
   if (rv == ERR_IO_PENDING) {
     // Synchronous operation not supported.
     DCHECK(!callback.is_null());
     // TODO(ajwong): Is setting read_callback_ the right thing to do here??
     read_callback_ = callback;
   } else {
     LogConnectCompletion(rv);
   }
@@ skipping 20 matching lines @@
         LOG(DFATAL) << "bad state " << state;
         rv = ERR_UNEXPECTED;
         break;
     }
   } while (rv != ERR_IO_PENDING && next_connect_state_ != CONNECT_STATE_NONE);
 
   return rv;
 }
 
 int TCPClientSocketWin::DoConnect() {
-  const struct addrinfo* ai = current_ai_;
-  DCHECK(ai);
+  DCHECK_LT(current_address_index_, addresses_.size());
   DCHECK_EQ(0, connect_os_error_);
 
+  const IPEndPoint& endpoint = addresses_[current_address_index_];
+
   if (previously_disconnected_) {
     use_history_.Reset();
     previously_disconnected_ = false;
   }
 
   net_log_.BeginEvent(NetLog::TYPE_TCP_CONNECT_ATTEMPT,
-                      new NetLogStringParameter(
-                          "address", NetAddressToStringWithPort(current_ai_)));
+                      new NetLogStringParameter("address",
+                                                endpoint.ToString()));
 
   next_connect_state_ = CONNECT_STATE_CONNECT_COMPLETE;
 
   if (bound_socket_ != INVALID_SOCKET) {
     DCHECK(bind_address_.get());
     socket_ = bound_socket_;
     bound_socket_ = INVALID_SOCKET;
   } else {
-    connect_os_error_ = CreateSocket(ai->ai_family, &socket_);
+    connect_os_error_ = CreateSocket(endpoint.GetFamily(), &socket_);

[eroman, 2012/05/04 01:08:41]

Side comment: When we refactor GetFamily() to return an enum rather than int we
need to be very careful about callsites like this, since they won't result in a
compile break. Might be worth changing the name while doing the refactor to
catch this.

[szym, 2012/05/04 02:38:29]

Yup. To avoid this pitfall when converting RequestPriority order I changed that
enum to a class to catch all call sites.

     if (connect_os_error_ != 0)
       return MapSystemError(connect_os_error_);
 
     if (bind_address_.get()) {
-      sockaddr_storage addr_storage;
-      sockaddr* addr = reinterpret_cast<struct sockaddr*>(&addr_storage);
-      size_t addr_len = sizeof(addr_storage);
-      if (!bind_address_->ToSockAddr(addr, &addr_len))
+      SockaddrStorage storage;
+      if (!bind_address_->ToSockAddr(storage.addr, &storage.addr_len))
         return ERR_INVALID_ARGUMENT;
-      if (bind(socket_, addr, addr_len))
+      if (bind(socket_, storage.addr, storage.addr_len))
         return MapSystemError(errno);
     }
   }
 
   DCHECK(!core_);
   core_ = new Core(this);
   // WSAEventSelect sets the socket to non-blocking mode as a side effect.
   // Our connect() and recv() calls require that the socket be non-blocking.
   WSAEventSelect(socket_, core_->read_overlapped_.hEvent, FD_CONNECT);
 
+  SockaddrStorage storage;
+  if (!endpoint.ToSockAddr(storage.addr, &storage.addr_len))
+    return ERR_INVALID_ARGUMENT;

[eroman, 2012/05/04 01:08:41]

I wander if these failure are possible? When building AddressList we should be
very explicit about what values are put in. I guess I need to read ToSockAddr to
see the failure cases.

Would be nice if this function didn't have any failures to check, and perhaps
just had a crash assertion if something really wonky happened.

[szym, 2012/05/04 02:38:29]

Agreed that ToSockAddr should ideally not return false. But even if we ignore
non-INET* junk, IPEndPoint could be in one of three states: empty, INET, INET6.
IPEndPoint().ToSockAddr(...) ?

   connect_start_time_ = base::TimeTicks::Now();
-  if (!connect(socket_, ai->ai_addr, static_cast<int>(ai->ai_addrlen))) {
+  if (!connect(socket_, storage.addr, storage.addr_len)) {
     // Connected without waiting!
     //
     // The MSDN page for connect says:
     //   With a nonblocking socket, the connection attempt cannot be completed
     //   immediately. In this case, connect will return SOCKET_ERROR, and
     //   WSAGetLastError will return WSAEWOULDBLOCK.
     // which implies that for a nonblocking socket, connect never returns 0.
     // It's not documented whether the event object will be signaled or not
     // if connect does return 0.  So the code below is essentially dead code
     // and we don't know if it's correct.
@@ skipping 26 matching lines @@
   if (result == OK) {
     connect_time_micros_ = base::TimeTicks::Now() - connect_start_time_;
     use_history_.set_was_ever_connected();
     return OK;  // Done!
   }
 
   // Close whatever partially connected socket we currently have.
   DoDisconnect();
 
   // Try to fall back to the next address in the list.
-  if (current_ai_->ai_next) {
+  if (current_address_index_ + 1 < addresses_.size()) {
     next_connect_state_ = CONNECT_STATE_CONNECT;
-    current_ai_ = current_ai_->ai_next;
+    ++current_address_index_;
     return OK;
   }
 
   // Otherwise there is nothing to fall back to, so give up.
   return result;
 }
 
 void TCPClientSocketWin::Disconnect() {
   DoDisconnect();
-  current_ai_ = NULL;
+  current_address_index_ = addresses_.size();

[eroman, 2012/05/04 01:08:41]

Same comment as earlier, I wander if this is actually necessary.

The only case I can see where this matters if if someone calls GetPeerAddress()
after disconnecting the socket.

However as this code stands, it will already early return based on not being
connected.

[szym, 2012/05/04 02:38:29]

Agreed. Will fix.

[eroman, 2012/05/04 19:37:36]

Sorry, I suggest leaving this in place, as a reset to -1.

 }
 
 void TCPClientSocketWin::DoDisconnect() {
   DCHECK(CalledOnValidThread());
 
   if (socket_ == INVALID_SOCKET)
     return;
 
   // Note: don't use CancelIo to cancel pending IO because it doesn't work
   // when there is a Winsock layered service provider.
@@ skipping 64 matching lines @@
     return false;
 
   return true;
 }
 
 int TCPClientSocketWin::GetPeerAddress(AddressList* address) const {
   DCHECK(CalledOnValidThread());
   DCHECK(address);
   if (!IsConnected())
     return ERR_SOCKET_NOT_CONNECTED;
-  *address = AddressList::CreateByCopyingFirstAddress(current_ai_);
+  *address = AddressList(addresses_[current_address_index_]);
   return OK;
 }
 
 int TCPClientSocketWin::GetLocalAddress(IPEndPoint* address) const {
   DCHECK(CalledOnValidThread());
   DCHECK(address);
   if (!IsConnected())
     return ERR_SOCKET_NOT_CONNECTED;
 
   struct sockaddr_storage addr_storage;
@@ skipping 265 matching lines @@
         use_history_.set_was_used_to_convey_data();
       net_log_.AddByteTransferEvent(NetLog::TYPE_SOCKET_BYTES_SENT, num_bytes,
                                     core_->write_buffer_.buf);
     }
   }
   core_->write_iobuffer_ = NULL;
   DoWriteCallback(rv);
 }
 
 }  // namespace net