syscall: refactor address APIs for deeper errno latching

include/envoy/network/address.h

@@ -7,7 +7,6 @@
 #include <cstdint>
 #include <memory>
 #include <string>
-#include <tuple>
 
 #include "envoy/common/pure.h"
 
@@ -17,6 +16,22 @@ namespace Envoy {
 namespace Network {
 namespace Address {
 
+/**
+ * Result holds the rc and errno values resulting from a system call.
+ */
+struct Result {
+
+  /**
+   * The return code from the system call.
+   */
+  int rc;
+
+  /**
+   * The errno value as captured after the system call.
+   */
+  int error;
+};
+
 /**
  * Interface for an Ipv4 address.
  */
@@ -129,19 +144,19 @@ class Instance {
    * Bind a socket to this address. The socket should have been created with a call to socket() on
    * an Instance of the same address family.
    * @param fd supplies the platform socket handle.
-   * @return a tuple of <0, errno> for success and <-1, errno> for failure. If the call is
-   *   successful, errno shouldn't be used.
+   * @return a Result with rc = 0 for success and rc = -1 for failure. If the call is successful,
+   *   the error value shouldn't be used.
    */
-  virtual std::tuple<int, int> bind(int fd) const PURE;
+  virtual Result bind(int fd) const PURE;
 
   /**
    * Connect a socket to this address. The socket should have been created with a call to socket()
    * on this object.
    * @param fd supplies the platform socket handle.
-   * @return a tuple of <0, errno> for success and <-1, errno> for failure. If the call is
-   *   successful, errno shouldn't be used.
+   * @return a Result with rc = 0 for success and rc = -1 for failure. If the call is successful,
+   *   the error value shouldn't be used.
    */
-  virtual std::tuple<int, int> connect(int fd) const PURE;
+  virtual Result connect(int fd) const PURE;
 
   /**
    * @return the IP address information IFF type() == Type::Ip, otherwise nullptr.
@@ -151,11 +166,11 @@ class Instance {
   /**
    * Create a socket for this address.
    * @param type supplies the socket type to create.
-   * @return a tuple of <fd, errno> for success and <-1, errno> for failure, where fd represents
-   *   the file descriptor naming the socket. If a valid file descriptor is returned, errno
+   * @return a Result with rc = fd for success and rc = -1 for failure, where fd represents the
+   *   file descriptor naming the socket. If a valid file descriptor is returned, the error value
    *   shouldn't be used.
    */
-  virtual std::tuple<int, int> socket(SocketType type) const PURE;
+  virtual Result socket(SocketType type) const PURE;
 
   /**
    * @return the type of address.

source/common/network/address_impl.cc

@@ -133,7 +133,7 @@ InstanceConstSharedPtr peerAddressFromFd(int fd) {
   return addressFromSockAddr(ss, ss_len);
 }
 
-std::tuple<int, int> InstanceBase::socketFromSocketType(SocketType socketType) const {
+Result InstanceBase::socketFromSocketType(SocketType socketType) const {
 #if defined(__APPLE__)
   int flags = 0;
 #else
@@ -168,7 +168,7 @@ std::tuple<int, int> InstanceBase::socketFromSocketType(SocketType socketType) c
   RELEASE_ASSERT(fcntl(fd, F_SETFL, O_NONBLOCK) != -1, "");
 #endif
 
-  return std::make_tuple(fd, errno);
+  return {fd, errno};
 }
 
 Ipv4Instance::Ipv4Instance(const sockaddr_in* address) : InstanceBase(Type::Ip) {
@@ -212,21 +212,19 @@ bool Ipv4Instance::operator==(const Instance& rhs) const {
           (ip_.port() == rhs_casted->ip_.port()));
 }
 
-std::tuple<int, int> Ipv4Instance::bind(int fd) const {
-  return std::make_tuple(::bind(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_),
-                                sizeof(ip_.ipv4_.address_)),
-                         errno);
+Result Ipv4Instance::bind(int fd) const {
+  return {::bind(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_),
+                 sizeof(ip_.ipv4_.address_)),
+          errno};
 }
 
-std::tuple<int, int> Ipv4Instance::connect(int fd) const {
-  return std::make_tuple(::connect(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_),
-                                   sizeof(ip_.ipv4_.address_)),
-                         errno);
+Result Ipv4Instance::connect(int fd) const {
+  return {::connect(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_),
+                    sizeof(ip_.ipv4_.address_)),
+          errno};
 }
 
-std::tuple<int, int> Ipv4Instance::socket(SocketType type) const {
-  return socketFromSocketType(type);
-}
+Result Ipv4Instance::socket(SocketType type) const { return socketFromSocketType(type); }
 
 absl::uint128 Ipv6Instance::Ipv6Helper::address() const {
   absl::uint128 result{0};
@@ -279,25 +277,24 @@ bool Ipv6Instance::operator==(const Instance& rhs) const {
           (ip_.port() == rhs_casted->ip_.port()));
 }
 
-std::tuple<int, int> Ipv6Instance::bind(int fd) const {
-  return std::make_tuple(::bind(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_),
-                                sizeof(ip_.ipv6_.address_)),
-                         errno);
+Result Ipv6Instance::bind(int fd) const {
+  return {::bind(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_),
+                 sizeof(ip_.ipv6_.address_)),
+          errno};
 }
 
-std::tuple<int, int> Ipv6Instance::connect(int fd) const {
-  return std::make_tuple(::connect(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_),
-                                   sizeof(ip_.ipv6_.address_)),
-                         errno);
+Result Ipv6Instance::connect(int fd) const {
+  return {::connect(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_),
+                    sizeof(ip_.ipv6_.address_)),
+          errno};
 }
 
-std::tuple<int, int> Ipv6Instance::socket(SocketType type) const {
-  const std::tuple<int, int> result = socketFromSocketType(type);
-  const int fd = std::get<0>(result);
-
+Result Ipv6Instance::socket(SocketType type) const {
+  const Result result = socketFromSocketType(type);
   // Setting IPV6_V6ONLY resticts the IPv6 socket to IPv6 connections only.
   const int v6only = ip_.v6only_;
-  RELEASE_ASSERT(::setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &v6only, sizeof(v6only)) != -1, "");
+  RELEASE_ASSERT(::setsockopt(result.rc, IPPROTO_IPV6, IPV6_V6ONLY, &v6only, sizeof(v6only)) != -1,
+                 "");
   return result;
 }
 
@@ -340,33 +337,29 @@ PipeInstance::PipeInstance(const std::string& pipe_path) : InstanceBase(Type::Pi
 
 bool PipeInstance::operator==(const Instance& rhs) const { return asString() == rhs.asString(); }
 
-std::tuple<int, int> PipeInstance::bind(int fd) const {
+Result PipeInstance::bind(int fd) const {
   if (abstract_namespace_) {
-    return std::make_tuple(::bind(fd, reinterpret_cast<const sockaddr*>(&address_),
-                                  offsetof(struct sockaddr_un, sun_path) + address_length_),
-                           errno);
+    return {::bind(fd, reinterpret_cast<const sockaddr*>(&address_),
+                   offsetof(struct sockaddr_un, sun_path) + address_length_),
+            errno};
   }
   // Try to unlink an existing filesystem object at the requested path. Ignore
   // errors -- it's fine if the path doesn't exist, and if it exists but can't
   // be unlinked then `::bind()` will generate a reasonable errno.
   unlink(address_.sun_path);
-  return std::make_tuple(::bind(fd, reinterpret_cast<const sockaddr*>(&address_), sizeof(address_)),
-                         errno);
+  return {::bind(fd, reinterpret_cast<const sockaddr*>(&address_), sizeof(address_)), errno};
 }
 
-std::tuple<int, int> PipeInstance::connect(int fd) const {
+Result PipeInstance::connect(int fd) const {
   if (abstract_namespace_) {
-    return std::make_tuple(::connect(fd, reinterpret_cast<const sockaddr*>(&address_),
-                                     offsetof(struct sockaddr_un, sun_path) + address_length_),
-                           errno);
+    return {::connect(fd, reinterpret_cast<const sockaddr*>(&address_),
+                      offsetof(struct sockaddr_un, sun_path) + address_length_),
+            errno};
   }
-  return std::make_tuple(
-      ::connect(fd, reinterpret_cast<const sockaddr*>(&address_), sizeof(address_)), errno);
+  return {::connect(fd, reinterpret_cast<const sockaddr*>(&address_), sizeof(address_)), errno};
 }
 
-std::tuple<int, int> PipeInstance::socket(SocketType type) const {
-  return socketFromSocketType(type);
-}
+Result PipeInstance::socket(SocketType type) const { return socketFromSocketType(type); }
 
 } // namespace Address
 } // namespace Network

source/common/network/address_impl.h

@@ -55,7 +55,7 @@ class InstanceBase : public Instance {
 
 protected:
   InstanceBase(Type type) : type_(type) {}
-  std::tuple<int, int> socketFromSocketType(SocketType type) const;
+  Result socketFromSocketType(SocketType type) const;
 
   std::string friendly_name_;
 
@@ -91,10 +91,10 @@ class Ipv4Instance : public InstanceBase {
 
   // Network::Address::Instance
   bool operator==(const Instance& rhs) const override;
-  std::tuple<int, int> bind(int fd) const override;
-  std::tuple<int, int> connect(int fd) const override;
+  Result bind(int fd) const override;
+  Result connect(int fd) const override;
   const Ip* ip() const override { return &ip_; }
-  std::tuple<int, int> socket(SocketType type) const override;
+  Result socket(SocketType type) const override;
 
 private:
   struct Ipv4Helper : public Ipv4 {
@@ -151,10 +151,10 @@ class Ipv6Instance : public InstanceBase {
 
   // Network::Address::Instance
   bool operator==(const Instance& rhs) const override;
-  std::tuple<int, int> bind(int fd) const override;
-  std::tuple<int, int> connect(int fd) const override;
+  Result bind(int fd) const override;
+  Result connect(int fd) const override;
   const Ip* ip() const override { return &ip_; }
-  std::tuple<int, int> socket(SocketType type) const override;
+  Result socket(SocketType type) const override;
 
 private:
   struct Ipv6Helper : public Ipv6 {
@@ -208,10 +208,10 @@ class PipeInstance : public InstanceBase {
 
   // Network::Address::Instance
   bool operator==(const Instance& rhs) const override;
-  std::tuple<int, int> bind(int fd) const override;
-  std::tuple<int, int> connect(int fd) const override;
+  Result bind(int fd) const override;
+  Result connect(int fd) const override;
   const Ip* ip() const override { return nullptr; }
-  std::tuple<int, int> socket(SocketType type) const override;
+  Result socket(SocketType type) const override;
 
 private:
   sockaddr_un address_;

source/common/network/connection_impl.cc

@@ -551,10 +551,10 @@ ClientConnectionImpl::ClientConnectionImpl(
   }
 
   if (source_address != nullptr) {
-    const std::tuple<int, int> result = source_address->bind(fd());
-    if (std::get<0>(result) < 0) {
+    Address::Result result = source_address->bind(fd());
+    if (result.rc < 0) {
       ENVOY_LOG_MISC(debug, "Bind failure. Failed to bind to {}: {}", source_address->asString(),
-                     strerror(std::get<1>(result)));
+                     strerror(result.error));
       bind_error_ = true;
       // Set a special error state to ensure asynchronous close to give the owner of the
       // ConnectionImpl a chance to add callbacks and detect the "disconnect".
@@ -568,21 +568,19 @@ ClientConnectionImpl::ClientConnectionImpl(
 
 void ClientConnectionImpl::connect() {
   ENVOY_CONN_LOG(debug, "connecting to {}", *this, socket_->remoteAddress()->asString());
-  const std::tuple<int, int> result = socket_->remoteAddress()->connect(fd());
-  const int rc = std::get<0>(result);
-  const int error = std::get<1>(result);
-  if (rc == 0) {
+  Address::Result result = socket_->remoteAddress()->connect(fd());
+  if (result.rc == 0) {
     // write will become ready.
     ASSERT(connecting_);
   } else {
-    ASSERT(rc == -1);
-    if (error == EINPROGRESS) {
+    ASSERT(result.rc == -1);
+    if (result.error == EINPROGRESS) {
       ASSERT(connecting_);
       ENVOY_CONN_LOG(debug, "connection in progress", *this);
     } else {
       immediate_error_event_ = ConnectionEvent::RemoteClose;
       connecting_ = false;
-      ENVOY_CONN_LOG(debug, "immediate connection error: {}", *this, error);
+      ENVOY_CONN_LOG(debug, "immediate connection error: {}", *this, result.error);
 
       // Trigger a write event. This is needed on OSX and seems harmless on Linux.
       file_event_->activate(Event::FileReadyType::Write);

source/common/network/listen_socket_impl.cc

@@ -16,11 +16,11 @@ namespace Envoy {
 namespace Network {
 
 void ListenSocketImpl::doBind() {
-  const std::tuple<int, int> result = local_address_->bind(fd_);
-  if (std::get<0>(result) == -1) {
+  const Address::Result result = local_address_->bind(fd_);
+  if (result.rc == -1) {
     close();
-    throw EnvoyException(fmt::format("cannot bind '{}': {}", local_address_->asString(),
-                                     strerror(std::get<1>(result))));
+    throw EnvoyException(
+        fmt::format("cannot bind '{}': {}", local_address_->asString(), strerror(result.error)));
   }
   if (local_address_->type() == Address::Type::Ip && local_address_->ip()->port() == 0) {
     // If the port we bind is zero, then the OS will pick a free port for us (assuming there are
@@ -39,7 +39,7 @@ void ListenSocketImpl::setListenSocketOptions(const Network::Socket::OptionsShar
 TcpListenSocket::TcpListenSocket(const Address::InstanceConstSharedPtr& address,
                                  const Network::Socket::OptionsSharedPtr& options,
                                  bool bind_to_port)
-    : ListenSocketImpl(std::get<0>(address->socket(Address::SocketType::Stream)), address) {
+    : ListenSocketImpl(address->socket(Address::SocketType::Stream).rc, address) {
   RELEASE_ASSERT(fd_ != -1, "");
 
   // TODO(htuch): This might benefit from moving to SocketOptionImpl.
@@ -61,7 +61,7 @@ TcpListenSocket::TcpListenSocket(int fd, const Address::InstanceConstSharedPtr&
 }
 
 UdsListenSocket::UdsListenSocket(const Address::InstanceConstSharedPtr& address)
-    : ListenSocketImpl(std::get<0>(address->socket(Address::SocketType::Stream)), address) {
+    : ListenSocketImpl(address->socket(Address::SocketType::Stream).rc, address) {
   RELEASE_ASSERT(fd_ != -1, "");
   doBind();
 }

source/common/network/listen_socket_impl.h

@@ -137,8 +137,8 @@ class AcceptedSocketImpl : public ConnectionSocketImpl {
 class ClientSocketImpl : public ConnectionSocketImpl {
 public:
   ClientSocketImpl(const Address::InstanceConstSharedPtr& remote_address)
-      : ConnectionSocketImpl(std::get<0>(remote_address->socket(Address::SocketType::Stream)),
-                             nullptr, remote_address) {}
+      : ConnectionSocketImpl(remote_address->socket(Address::SocketType::Stream).rc, nullptr,
+                             remote_address) {}
 };
 
 } // namespace Network

source/extensions/stat_sinks/common/statsd/statsd.cc

@@ -20,10 +20,10 @@ namespace Common {
 namespace Statsd {
 
 Writer::Writer(Network::Address::InstanceConstSharedPtr address) {
-  fd_ = std::get<0>(address->socket(Network::Address::SocketType::Datagram));
+  fd_ = address->socket(Network::Address::SocketType::Datagram).rc;
   ASSERT(fd_ != -1);
 
-  const int rc = std::get<0>(address->connect(fd_));
+  const int rc = address->connect(fd_).rc;
   ASSERT(rc != -1);
 }