syscall: refactor address APIs for deeper errno latching

include/envoy/network/address.h

@@ -7,6 +7,7 @@
 #include <cstdint>
 #include <memory>
 #include <string>
+#include <tuple>
 
 #include "envoy/common/pure.h"
 
@@ -128,19 +129,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 0 for success and -1 for failure. The error code associated with a failure will
-   * be accessible in a plaform dependent fashion (e.g. errno for Unix platforms).
+   * @return a tuple of <0, errno> for success and <-1, errno> for failure. If the call is
+   *   successful, errno shouldn't be used.
    */
-  virtual int bind(int fd) const PURE;
+  virtual std::tuple<int, int> 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 0 for success and -1 for failure. The error code associated with a failure will
-   * be accessible in a plaform dependent fashion (e.g. errno for Unix platforms).
+   * @return a tuple of <0, errno> for success and <-1, errno> for failure. If the call is
+   *   successful, errno shouldn't be used.
    */
-  virtual int connect(int fd) const PURE;
+  virtual std::tuple<int, int> connect(int fd) const PURE;
 
   /**
    * @return the IP address information IFF type() == Type::Ip, otherwise nullptr.
@@ -150,11 +151,11 @@ class Instance {
   /**
    * Create a socket for this address.
    * @param type supplies the socket type to create.
-   * @return the file descriptor naming the socket for success and -1 for failure. The error
-   * code associated with a failure will be accessible in a plaform dependent fashion (e.g.
-   * errno for Unix platforms).
+   * @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
+   *   shouldn't be used.
    */
-  virtual int socket(SocketType type) const PURE;
+  virtual std::tuple<int, int> 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);
 }
 
-int InstanceBase::socketFromSocketType(SocketType socketType) const {
+std::tuple<int, int> InstanceBase::socketFromSocketType(SocketType socketType) const {
 #if defined(__APPLE__)
   int flags = 0;
 #else
@@ -168,7 +168,7 @@ int InstanceBase::socketFromSocketType(SocketType socketType) const {
   RELEASE_ASSERT(fcntl(fd, F_SETFL, O_NONBLOCK) != -1, "");
 #endif
 
-  return fd;
+  return std::make_tuple(fd, errno);
 }
 
 Ipv4Instance::Ipv4Instance(const sockaddr_in* address) : InstanceBase(Type::Ip) {
@@ -212,17 +212,21 @@ bool Ipv4Instance::operator==(const Instance& rhs) const {
           (ip_.port() == rhs_casted->ip_.port()));
 }
 
-int Ipv4Instance::bind(int fd) const {
-  return ::bind(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_),
-                sizeof(ip_.ipv4_.address_));
+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);
 }
 
-int Ipv4Instance::connect(int fd) const {
-  return ::connect(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_),
-                   sizeof(ip_.ipv4_.address_));
+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);
 }
 
-int Ipv4Instance::socket(SocketType type) const { return socketFromSocketType(type); }
+std::tuple<int, int> Ipv4Instance::socket(SocketType type) const {
+  return socketFromSocketType(type);
+}
 
 absl::uint128 Ipv6Instance::Ipv6Helper::address() const {
   absl::uint128 result{0};
@@ -275,23 +279,26 @@ bool Ipv6Instance::operator==(const Instance& rhs) const {
           (ip_.port() == rhs_casted->ip_.port()));
 }
 
-int Ipv6Instance::bind(int fd) const {
-  return ::bind(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_),
-                sizeof(ip_.ipv6_.address_));
+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);
 }
 
-int Ipv6Instance::connect(int fd) const {
-  return ::connect(fd, reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_),
-                   sizeof(ip_.ipv6_.address_));
+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);
 }
 
-int Ipv6Instance::socket(SocketType type) const {
-  const int fd = socketFromSocketType(type);
+std::tuple<int, int> Ipv6Instance::socket(SocketType type) const {
+  const std::tuple<int, int> result = socketFromSocketType(type);
+  const int fd = std::get<0>(result);
 
   // 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, "");
-  return fd;
+  return result;
 }
 
 PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len)
@@ -333,27 +340,33 @@ PipeInstance::PipeInstance(const std::string& pipe_path) : InstanceBase(Type::Pi
 
 bool PipeInstance::operator==(const Instance& rhs) const { return asString() == rhs.asString(); }
 
-int PipeInstance::bind(int fd) const {
+std::tuple<int, int> PipeInstance::bind(int fd) const {
   if (abstract_namespace_) {
-    return ::bind(fd, reinterpret_cast<const sockaddr*>(&address_),
-                  offsetof(struct sockaddr_un, sun_path) + address_length_);
+    return std::make_tuple(::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 ::bind(fd, reinterpret_cast<const sockaddr*>(&address_), sizeof(address_));
+  return std::make_tuple(::bind(fd, reinterpret_cast<const sockaddr*>(&address_), sizeof(address_)),
+                         errno);
 }
 
-int PipeInstance::connect(int fd) const {
+std::tuple<int, int> PipeInstance::connect(int fd) const {
   if (abstract_namespace_) {
-    return ::connect(fd, reinterpret_cast<const sockaddr*>(&address_),
-                     offsetof(struct sockaddr_un, sun_path) + address_length_);
+    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_), sizeof(address_));
+  return std::make_tuple(
+      ::connect(fd, reinterpret_cast<const sockaddr*>(&address_), sizeof(address_)), errno);
 }
 
-int PipeInstance::socket(SocketType type) const { return socketFromSocketType(type); }
+std::tuple<int, int> 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) {}
-  int socketFromSocketType(SocketType type) const;
+  std::tuple<int, int> 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;
-  int bind(int fd) const override;
-  int connect(int fd) const override;
+  std::tuple<int, int> bind(int fd) const override;
+  std::tuple<int, int> connect(int fd) const override;
   const Ip* ip() const override { return &ip_; }
-  int socket(SocketType type) const override;
+  std::tuple<int, int> 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;
-  int bind(int fd) const override;
-  int connect(int fd) const override;
+  std::tuple<int, int> bind(int fd) const override;
+  std::tuple<int, int> connect(int fd) const override;
   const Ip* ip() const override { return &ip_; }
-  int socket(SocketType type) const override;
+  std::tuple<int, int> 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;
-  int bind(int fd) const override;
-  int connect(int fd) const override;
+  std::tuple<int, int> bind(int fd) const override;
+  std::tuple<int, int> connect(int fd) const override;
   const Ip* ip() const override { return nullptr; }
-  int socket(SocketType type) const override;
+  std::tuple<int, int> 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 int rc = source_address->bind(fd());
-    if (rc < 0) {
+    const std::tuple<int, int> result = source_address->bind(fd());
+    if (std::get<0>(result) < 0) {
       ENVOY_LOG_MISC(debug, "Bind failure. Failed to bind to {}: {}", source_address->asString(),
-                     strerror(errno));
+                     strerror(std::get<1>(result)));
       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,19 +568,21 @@ ClientConnectionImpl::ClientConnectionImpl(
 
 void ClientConnectionImpl::connect() {
   ENVOY_CONN_LOG(debug, "connecting to {}", *this, socket_->remoteAddress()->asString());
-  const int rc = socket_->remoteAddress()->connect(fd());
+  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) {
     // write will become ready.
     ASSERT(connecting_);
   } else {
     ASSERT(rc == -1);
-    if (errno == EINPROGRESS) {
+    if (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, errno);
+      ENVOY_CONN_LOG(debug, "immediate connection error: {}", *this, 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() {
-  int rc = local_address_->bind(fd_);
-  if (rc == -1) {
+  const std::tuple<int, int> result = local_address_->bind(fd_);
+  if (std::get<0>(result) == -1) {
     close();
-    throw EnvoyException(
-        fmt::format("cannot bind '{}': {}", local_address_->asString(), strerror(errno)));
+    throw EnvoyException(fmt::format("cannot bind '{}': {}", local_address_->asString(),
+                                     strerror(std::get<1>(result))));
   }
   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(address->socket(Address::SocketType::Stream), address) {
+    : ListenSocketImpl(std::get<0>(address->socket(Address::SocketType::Stream)), 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(address->socket(Address::SocketType::Stream), address) {
+    : ListenSocketImpl(std::get<0>(address->socket(Address::SocketType::Stream)), 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(remote_address->socket(Address::SocketType::Stream), nullptr,
-                             remote_address) {}
+      : ConnectionSocketImpl(std::get<0>(remote_address->socket(Address::SocketType::Stream)),
+                             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_ = address->socket(Network::Address::SocketType::Datagram);
+  fd_ = std::get<0>(address->socket(Network::Address::SocketType::Datagram));
   ASSERT(fd_ != -1);
 
-  int rc = address->connect(fd_);
+  const int rc = std::get<0>(address->connect(fd_));
   ASSERT(rc != -1);
 }
 

test/common/network/addr_family_aware_socket_option_impl_test.cc

@@ -23,7 +23,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, SetOptionFailure) {
 // If a platform supports IPv4 socket option variant for an IPv4 address, it works
 TEST_F(AddrFamilyAwareSocketOptionImplTest, SetOptionSuccess) {
   Address::Ipv4Instance address("1.2.3.4", 5678);
-  const int fd = address.socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(address.socket(Address::SocketType::Stream));
   EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
 
   AddrFamilyAwareSocketOptionImpl socket_option{envoy::api::v2::core::SocketOption::STATE_PREBIND,
@@ -37,7 +37,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, SetOptionSuccess) {
 // If a platform doesn't support IPv4 socket option variant for an IPv4 address we fail
 TEST_F(AddrFamilyAwareSocketOptionImplTest, V4EmptyOptionNames) {
   Address::Ipv4Instance address("1.2.3.4", 5678);
-  const int fd = address.socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(address.socket(Address::SocketType::Stream));
   EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
   AddrFamilyAwareSocketOptionImpl socket_option{
       envoy::api::v2::core::SocketOption::STATE_PREBIND, {}, {}, 1};
@@ -50,7 +50,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V4EmptyOptionNames) {
 // If a platform doesn't support IPv4 and IPv6 socket option variants for an IPv4 address, we fail
 TEST_F(AddrFamilyAwareSocketOptionImplTest, V6EmptyOptionNames) {
   Address::Ipv6Instance address("::1:2:3:4", 5678);
-  const int fd = address.socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(address.socket(Address::SocketType::Stream));
   EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
   AddrFamilyAwareSocketOptionImpl socket_option{
       envoy::api::v2::core::SocketOption::STATE_PREBIND, {}, {}, 1};
@@ -64,7 +64,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V6EmptyOptionNames) {
 // IPv4 varient
 TEST_F(AddrFamilyAwareSocketOptionImplTest, V4IgnoreV6) {
   Address::Ipv4Instance address("1.2.3.4", 5678);
-  const int fd = address.socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(address.socket(Address::SocketType::Stream));
   EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
 
   AddrFamilyAwareSocketOptionImpl socket_option{envoy::api::v2::core::SocketOption::STATE_PREBIND,
@@ -78,7 +78,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V4IgnoreV6) {
 // If a platform suppports IPv6 socket option variant for an IPv6 address it works
 TEST_F(AddrFamilyAwareSocketOptionImplTest, V6Only) {
   Address::Ipv6Instance address("::1:2:3:4", 5678);
-  const int fd = address.socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(address.socket(Address::SocketType::Stream));
   EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
 
   AddrFamilyAwareSocketOptionImpl socket_option{envoy::api::v2::core::SocketOption::STATE_PREBIND,
@@ -93,7 +93,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V6Only) {
 // we apply the IPv4 variant.
 TEST_F(AddrFamilyAwareSocketOptionImplTest, V6OnlyV4Fallback) {
   Address::Ipv6Instance address("::1:2:3:4", 5678);
-  const int fd = address.socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(address.socket(Address::SocketType::Stream));
   EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
 
   AddrFamilyAwareSocketOptionImpl socket_option{envoy::api::v2::core::SocketOption::STATE_PREBIND,
@@ -108,7 +108,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V6OnlyV4Fallback) {
 // AddrFamilyAwareSocketOptionImpl::setIpSocketOption() works with the IPv6 variant.
 TEST_F(AddrFamilyAwareSocketOptionImplTest, V6Precedence) {
   Address::Ipv6Instance address("::1:2:3:4", 5678);
-  const int fd = address.socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(address.socket(Address::SocketType::Stream));
   EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
 
   AddrFamilyAwareSocketOptionImpl socket_option{envoy::api::v2::core::SocketOption::STATE_PREBIND,