Fix TCPClientSocket::GetLocalAddress() to work when socket is not connected.

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"
@@ skipping 355 matching lines @@
 int TCPClientSocketWin::Bind(const IPEndPoint& address) {
   if (current_address_index_ >= 0 || bind_address_.get()) {
     // Cannot bind the socket if we are already connected or connecting.
     return ERR_UNEXPECTED;
   }
 
   SockaddrStorage storage;
   if (!address.ToSockAddr(storage.addr, &storage.addr_len))
     return ERR_INVALID_ARGUMENT;
 
-  // Create |bound_socket_| and try to bound it to |address|.
+  // Create |bound_socket_| and try to bind it to |address|.
   int error = CreateSocket(address.GetFamily(), &bound_socket_);
   if (error)
     return MapSystemError(error);
 
   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);
@@ skipping 252 matching lines @@
   DCHECK(address);
   if (!IsConnected())
     return ERR_SOCKET_NOT_CONNECTED;
   *address = addresses_[current_address_index_];
   return OK;
 }
 
 int TCPClientSocketWin::GetLocalAddress(IPEndPoint* address) const {
   DCHECK(CalledOnValidThread());
   DCHECK(address);
-  if (!IsConnected())
-    return ERR_SOCKET_NOT_CONNECTED;
+  if (socket_ == INVALID_SOCKET) {
+    if (bind_address_.get()) {
+      *address = *bind_address_;
+      return OK;
+    } else {

[wtc, 2012/07/19 18:36:36]

Nit: omit "else" after a return statement.

[Sergey Ulanov, 2012/07/19 21:36:20]

Done.

+      return ERR_SOCKET_NOT_CONNECTED;
+    }
+  }

[wtc, 2012/07/19 18:36:36]

If socket_ is INVALID_SOCKET, I think we should simply
return an error.  Returning ERR_SOCKET_NOT_CONNECTED in
this case is fine.  We probably should add a new error
code ERR_SOCKET_NOT_BOUND.

  if (socket_ == INVALID_SOCKET)
    return ERR_SOCKET_NOT_CONNECTED;

The !IsConnected() test is inappropriate because it can
become true by the peer's action alone (if the peer closes
the TCP connection).  On the other hand, socket_ can only
be INVALID_SOCKET by our local action (if we have never
called Connect, or have called Disconnect).

BUT, your unit test showed the semantics you want to support.
I think your semantics is reasonable, too.  In that case,
I think Disconnect must clear bind_address_.

Finally, the comment for the GetLocalAddress method in
net/socket/stream_socket.h should be updated to describe
the new behavior.

[Sergey Ulanov, 2012/07/19 21:36:20]

Done.
Done.

 
   struct sockaddr_storage addr_storage;
   socklen_t addr_len = sizeof(addr_storage);
   struct sockaddr* addr = reinterpret_cast<struct sockaddr*>(&addr_storage);
   if (getsockname(socket_, addr, &addr_len))
     return MapSystemError(WSAGetLastError());
   if (!address->FromSockAddr(addr, addr_len))
     return ERR_FAILED;
   return OK;
 }
@@ skipping 280 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