Update address's socket method signature

This commit ensures that address implementations return the errno along
with the file descriptor in socket method calls.

Signed-off-by: Venil Noronha <veniln@vmware.com>

include/envoy/network/address.h

@@ -151,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) {
@@ -224,7 +224,9 @@ std::tuple<int, int> Ipv4Instance::connect(int fd) const {
                          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};
@@ -289,13 +291,14 @@ std::tuple<int, int> Ipv6Instance::connect(int fd) const {
                          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)
@@ -361,7 +364,9 @@ std::tuple<int, int> PipeInstance::connect(int fd) const {
       ::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_;
 
@@ -94,7 +94,7 @@ class Ipv4Instance : public InstanceBase {
   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 {
@@ -154,7 +154,7 @@ class Ipv6Instance : public InstanceBase {
   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 {
@@ -211,7 +211,7 @@ class PipeInstance : public InstanceBase {
   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/listen_socket_impl.cc

@@ -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,7 +20,7 @@ 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);
 
   const int rc = std::get<0>(address->connect(fd_));

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,

test/common/network/address_impl_test.cc

@@ -51,7 +51,7 @@ void testSocketBindAndConnect(Network::Address::IpVersion ip_version, bool v6onl
   ASSERT_NE(addr_port->ip(), nullptr);
 
   // Create a socket on which we'll listen for connections from clients.
-  const int listen_fd = addr_port->socket(SocketType::Stream);
+  const int listen_fd = std::get<0>(addr_port->socket(SocketType::Stream));
   ASSERT_GE(listen_fd, 0) << addr_port->asString();
   ScopedFdCloser closer1(listen_fd);
 
@@ -75,7 +75,7 @@ void testSocketBindAndConnect(Network::Address::IpVersion ip_version, bool v6onl
 
   auto client_connect = [](Address::InstanceConstSharedPtr addr_port) {
     // Create a client socket and connect to the server.
-    const int client_fd = addr_port->socket(SocketType::Stream);
+    const int client_fd = std::get<0>(addr_port->socket(SocketType::Stream));
     ASSERT_GE(client_fd, 0) << addr_port->asString();
     ScopedFdCloser closer2(client_fd);
 
@@ -312,7 +312,7 @@ TEST(PipeInstanceTest, BadAddress) {
 TEST(PipeInstanceTest, UnlinksExistingFile) {
   const auto bind_uds_socket = [](const std::string& path) {
     PipeInstance address(path);
-    const int listen_fd = address.socket(SocketType::Stream);
+    const int listen_fd = std::get<0>(address.socket(SocketType::Stream));
     ASSERT_GE(listen_fd, 0) << address.asString();
     ScopedFdCloser closer(listen_fd);
 

test/common/network/dns_impl_test.cc

@@ -362,7 +362,9 @@ class CustomInstance : public Address::Instance {
   std::tuple<int, int> bind(int fd) const override { return instance_.bind(fd); }
   std::tuple<int, int> connect(int fd) const override { return instance_.connect(fd); }
   const Address::Ip* ip() const override { return instance_.ip(); }
-  int socket(Address::SocketType type) const override { return instance_.socket(type); }
+  std::tuple<int, int> socket(Address::SocketType type) const override {
+    return instance_.socket(type);
+  }
   Address::Type type() const override { return instance_.type(); }
 
 private:

test/mocks/network/mocks.h

@@ -412,7 +412,7 @@ class MockResolvedAddress : public Address::Instance {
   MOCK_CONST_METHOD1(bind, std::tuple<int, int>(int));
   MOCK_CONST_METHOD1(connect, std::tuple<int, int>(int));
   MOCK_CONST_METHOD0(ip, Address::Ip*());
-  MOCK_CONST_METHOD1(socket, int(Address::SocketType));
+  MOCK_CONST_METHOD1(socket, std::tuple<int, int>(Address::SocketType));
   MOCK_CONST_METHOD0(type, Address::Type());
 
   const std::string& asString() const override { return physical_; }

test/test_common/network_utility.cc

@@ -26,9 +26,11 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstShared
                   << (addr_port == nullptr ? "nullptr" : addr_port->asString());
     return nullptr;
   }
-  const int fd = addr_port->socket(type);
+  std::tuple<int, int> result = addr_port->socket(type);
+  const int fd = std::get<0>(result);
+  int err;
   if (fd < 0) {
-    const int err = errno;
+    err = std::get<1>(result);
     ADD_FAILURE() << "socket failed for '" << addr_port->asString()
                   << "' with error: " << strerror(err) << " (" << err << ")";
     return nullptr;
@@ -37,9 +39,8 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstShared
   // Not setting REUSEADDR, therefore if the address has been recently used we won't reuse it here.
   // However, because we're going to use the address while checking if it is available, we'll need
   // to set REUSEADDR on listener sockets created by tests using an address validated by this means.
-  std::tuple<int, int> result = addr_port->bind(fd);
+  result = addr_port->bind(fd);
   int rc = std::get<0>(result);
-  int err;
   const char* failing_fn = nullptr;
   if (rc != 0) {
     err = std::get<1>(result);
@@ -150,7 +151,7 @@ Address::InstanceConstSharedPtr getAnyAddress(const Address::IpVersion version,
 
 bool supportsIpVersion(const Address::IpVersion version) {
   Address::InstanceConstSharedPtr addr = getCanonicalLoopbackAddress(version);
-  const int fd = addr->socket(Address::SocketType::Stream);
+  const int fd = std::get<0>(addr->socket(Address::SocketType::Stream));
   if (fd < 0) {
     // Socket creation failed.
     return false;
@@ -168,13 +169,14 @@ std::pair<Address::InstanceConstSharedPtr, int> bindFreeLoopbackPort(Address::Ip
                                                                      Address::SocketType type) {
   Address::InstanceConstSharedPtr addr = getCanonicalLoopbackAddress(version);
   const char* failing_fn = nullptr;
-  const int fd = addr->socket(type);
+  std::tuple<int, int> result = addr->socket(type);
+  const int fd = std::get<0>(result);
   int err;
   if (fd < 0) {
-    err = errno;
+    err = std::get<1>(result);
     failing_fn = "socket";
   } else {
-    std::tuple<int, int> result = addr->bind(fd);
+    result = addr->bind(fd);
     if (0 != std::get<0>(result)) {
       err = std::get<1>(result);
       failing_fn = "bind";