Adapt python scripts to use the new Forwarder2

tools/android/forwarder2/socket.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 "tools/android/forwarder2/socket.h"
 
 #include <arpa/inet.h>
 #include <fcntl.h>
 #include <netdb.h>
 #include <netinet/in.h>
@@ skipping 111 matching lines @@
   // For abstract sockets we need one extra byte for the leading zero.
   if ((abstract && path.size() + 2 /* '\0' */ > kPathMax) ||
       (!abstract && path.size() + 1 /* '\0' */ > kPathMax)) {
     LOG(ERROR) << "The provided path is too big to create a unix "
                << "domain socket: " << path;
     return false;
   }
   abstract_ = abstract;
   family_ = PF_UNIX;
   addr_.addr_un.sun_family = family_;
-
   if (abstract) {
     // Copied from net/base/unix_domain_socket_posix.cc
     // Convert the path given into abstract socket name. It must start with
     // the '\0' character, so we are adding it. |addr_len| must specify the
     // length of the structure exactly, as potentially the socket name may
     // have '\0' characters embedded (although we don't support this).
     // Note that addr_.addr_un.sun_path is already zero initialized.
     memcpy(addr_.addr_un.sun_path + 1, path.c_str(), path.size());
     addr_len_ = path.size() + offsetof(struct sockaddr_un, sun_path) + 1;
   } else {
     memcpy(addr_.addr_un.sun_path, path.c_str(), path.size());
     addr_len_ = sizeof(sockaddr_un);
   }
-
   addr_ptr_ = reinterpret_cast<sockaddr*>(&addr_.addr_un);
   return InitSocketInternal();
 }
 
 bool Socket::InitTcpSocket(const std::string& host, int port) {
   port_ = port;
-
   if (host.empty()) {
     // Use localhost: INADDR_LOOPBACK
     family_ = AF_INET;
     addr_.addr4.sin_family = family_;
     addr_.addr4.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
   } else if (!Resolve(host)) {
     return false;
   }
   CHECK(family_ == AF_INET || family_ == AF_INET6)
       << "Invalid socket family.";
   if (family_ == AF_INET) {
     addr_.addr4.sin_port = htons(port_);
     addr_ptr_ = reinterpret_cast<sockaddr*>(&addr_.addr4);
     addr_len_ = sizeof(addr_.addr4);
   } else if (family_ == AF_INET6) {
     addr_.addr6.sin6_port = htons(port_);
     addr_ptr_ = reinterpret_cast<sockaddr*>(&addr_.addr6);
     addr_len_ = sizeof(addr_.addr6);
   }
   return InitSocketInternal();
 }
 
 bool Socket::BindAndListen() {
   errno = 0;
   if (HANDLE_EINTR(bind(socket_, addr_ptr_, addr_len_)) < 0 ||
       HANDLE_EINTR(listen(socket_, 5)) < 0) {
     SetSocketError();
     return false;
   }
-  if (port_ == 0) {
+  if (port_ == 0 && (family_ == AF_INET || family_ == AF_INET6)) {

[Philippe, 2012/09/26 00:15:46]

Nit: I think that this could be factored out into a helper method since you use
it in multiple places. 

[felipeg, 2012/09/26 00:34:28]

Done.

     SockAddr addr;
     memset(&addr, 0, sizeof(addr));
     socklen_t addrlen = 0;
     sockaddr* addr_ptr = NULL;
     uint16* port_ptr = NULL;
     if (family_ == AF_INET) {
       addr_ptr = reinterpret_cast<sockaddr*>(&addr.addr4);
       port_ptr = &addr.addr4.sin_port;
       addrlen = sizeof(addr.addr4);
     } else if (family_ == AF_INET6) {
@@ skipping 24 matching lines @@
     SetSocketError();
     return false;
   }
 
   tools::DisableNagle(new_socket_fd);
   new_socket->socket_ = new_socket_fd;
   return true;
 }
 
 bool Socket::Connect() {
-  // Set non-block because we use select.
+  // Set non-block because we use select for connect.
   fcntl(socket_, F_SETFL, fcntl(socket_, F_GETFL) | O_NONBLOCK);

[Philippe, 2012/09/26 00:15:46]

Nit: maybe store the result of fcntl() into a const variable like
|non_block_flags| then use it on line 239.
Maybe add an extra DCHECK(!(non_block_flags & O_NONBLOCK)) too.

[felipeg, 2012/09/26 00:34:28]

Done.

   errno = 0;
   if (HANDLE_EINTR(connect(socket_, addr_ptr_, addr_len_)) < 0 &&
       errno != EINPROGRESS) {
     SetSocketError();
     return false;

[digit1, 2012/09/26 00:19:04]

you should restore the flags before returning.

[Philippe, 2012/09/26 00:30:10]

Good point.

This might be a good opportunity to use RAII since we do multiple returns.

Something like this at the beginning of the scope:

struct RestoreFlags {
  RestoreFlags(old_flags) : old_flags_(old_flags) {}
  ~RestoreFlags() { fcntl(socket_, F_SETFL, old_flags_); }
  const int old_flags_;
} restore_flags(fcntl(socket_, F_GETFL));

This might be overkill though :)

base/auto_reset.h provides this kind of pattern, unfortunately it doesn't let us
specify how we restore the value.

[felipeg, 2012/09/26 00:34:28]

Done.

   }
   // Wait for connection to complete, or receive a notification.
   if (!WaitForEvent(WRITE, kConnectTimeOut)) {
     SetSocketError();
     return false;
   }
+  // Disable non-block since our code assumes blocking semantics.
+  fcntl(socket_, F_SETFL, fcntl(socket_, F_GETFL) & ~O_NONBLOCK);

[digit1, 2012/09/26 00:19:04]

I agree with Philippe that this kind of assumption can lead to really big
surprises. I.e. it's probably simpler to save the old socket flags in a local
variable, and restore them on exit. You will have to do that even in case of
error (i.e. the "return false" cases above), and preserve the errno.

[felipeg, 2012/09/26 00:34:28]

Done.

   return true;
 }
 
 bool Socket::Resolve(const std::string& host) {
   struct addrinfo hints;
   struct addrinfo* res;
   memset(&hints, 0, sizeof(hints));
   hints.ai_family = AF_UNSPEC;
   hints.ai_socktype = SOCK_STREAM;
   hints.ai_flags |= AI_CANONNAME;
@@ skipping 117 matching lines @@
   return !FD_ISSET(exit_notifier_fd_, &read_fds);
 }
 
 // static
 int Socket::GetHighestFileDescriptor(
     const Socket& s1, const Socket& s2) {
   return std::max(s1.socket_, s2.socket_);
 }
 
 }  // namespace forwarder