Skip to content

Commit

Permalink
Replace std::tuple<int, int> with Result
Browse files Browse the repository at this point in the history
This commit adds a struct named Result which holds the rc and errno
values resulting from a syscall. Also, the address API is refactored to
use the new Result struct instead of std::tuple<int, int>.

Signed-off-by: Venil Noronha <veniln@vmware.com>
  • Loading branch information
venilnoronha committed Jul 24, 2018
1 parent 6c65b13 commit 36c81e5
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 128 deletions.
35 changes: 25 additions & 10 deletions include/envoy/network/address.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <cstdint>
#include <memory>
#include <string>
#include <tuple>

#include "envoy/common/pure.h"

Expand All @@ -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.
*/
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
75 changes: 34 additions & 41 deletions source/common/network/address_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions source/common/network/address_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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_;
Expand Down
18 changes: 8 additions & 10 deletions source/common/network/connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions source/common/network/listen_socket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions source/common/network/listen_socket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions source/extensions/stat_sinks/common/statsd/statsd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Loading

0 comments on commit 36c81e5

Please sign in to comment.