From aead221184f52e68d161118a925a242f0858f736 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 28 Oct 2016 10:36:05 +0200 Subject: [PATCH] event/ServerSocket: migrate from class Error to C++ exceptions --- src/Listen.cxx | 64 +++++----- src/Listen.hxx | 5 +- src/Main.cxx | 6 +- src/event/ServerSocket.cxx | 116 +++++++----------- src/event/ServerSocket.hxx | 26 ++-- src/net/Resolver.cxx | 17 +-- src/net/Resolver.hxx | 11 +- src/net/SocketError.hxx | 19 +++ src/net/SocketUtil.cxx | 24 ++-- src/net/SocketUtil.hxx | 8 +- src/output/plugins/httpd/HttpdInternal.hxx | 2 +- .../plugins/httpd/HttpdOutputPlugin.cxx | 38 +++--- test/run_resolver.cxx | 16 ++- 13 files changed, 164 insertions(+), 188 deletions(-) diff --git a/src/Listen.cxx b/src/Listen.cxx index 6110c0dfc..7c648cf40 100644 --- a/src/Listen.cxx +++ b/src/Listen.cxx @@ -25,7 +25,8 @@ #include "config/ConfigOption.hxx" #include "net/SocketAddress.hxx" #include "event/ServerSocket.hxx" -#include "util/Error.hxx" +#include "system/Error.hxx" +#include "util/RuntimeError.hxx" #include "util/Domain.hxx" #include "fs/AllocatedPath.hxx" #include "Log.hxx" @@ -58,50 +59,47 @@ private: static ClientListener *listen_socket; int listen_port; -static bool +/** + * Throws #std::runtime_error on error. + */ +static void listen_add_config_param(unsigned int port, - const ConfigParam *param, - Error &error_r) + const ConfigParam *param) { assert(param != nullptr); if (0 == strcmp(param->value.c_str(), "any")) { - return listen_socket->AddPort(port, error_r); + listen_socket->AddPort(port); } else if (param->value[0] == '/' || param->value[0] == '~') { - auto path = param->GetPath(error_r); - return !path.IsNull() && - listen_socket->AddPath(std::move(path), error_r); + listen_socket->AddPath(param->GetPath()); } else { - return listen_socket->AddHost(param->value.c_str(), port, - error_r); + listen_socket->AddHost(param->value.c_str(), port); } } #ifdef ENABLE_SYSTEMD_DAEMON static bool -listen_systemd_activation(Error &error_r) +listen_systemd_activation() { int n = sd_listen_fds(true); if (n <= 0) { if (n < 0) - FormatErrno(listen_domain, -n, - "sd_listen_fds() failed"); + throw MakeErrno(-n, "sd_listen_fds() failed"); return false; } for (int i = SD_LISTEN_FDS_START, end = SD_LISTEN_FDS_START + n; i != end; ++i) - if (!listen_socket->AddFD(i, error_r)) - return false; + listen_socket->AddFD(i); return true; } #endif -bool -listen_global_init(EventLoop &loop, Partition &partition, Error &error) +void +listen_global_init(EventLoop &loop, Partition &partition) { int port = config_get_positive(ConfigOption::PORT, DEFAULT_PORT); const auto *param = config_get_param(ConfigOption::BIND_TO_ADDRESS); @@ -109,11 +107,8 @@ listen_global_init(EventLoop &loop, Partition &partition, Error &error) listen_socket = new ClientListener(loop, partition); #ifdef ENABLE_SYSTEMD_DAEMON - if (listen_systemd_activation(error)) - return true; - - if (error.IsDefined()) - return false; + if (listen_systemd_activation()) + return; #endif if (param != nullptr) { @@ -121,32 +116,35 @@ listen_global_init(EventLoop &loop, Partition &partition, Error &error) for all values */ do { - if (!listen_add_config_param(port, param, error)) { + try { + listen_add_config_param(port, param); + } catch (const std::runtime_error &e) { delete listen_socket; - error.FormatPrefix("Failed to listen on %s (line %i): ", - param->value.c_str(), - param->line); - return false; + std::throw_with_nested(FormatRuntimeError("Failed to listen on %s (line %i)", + param->value.c_str(), + param->line)); } } while ((param = param->next) != nullptr); } else { /* no "bind_to_address" configured, bind the configured port on all interfaces */ - if (!listen_socket->AddPort(port, error)) { + try { + listen_socket->AddPort(port); + } catch (const std::runtime_error &e) { delete listen_socket; - error.FormatPrefix("Failed to listen on *:%d: ", port); - return false; + std::throw_with_nested(FormatRuntimeError("Failed to listen on *:%d: ", port)); } } - if (!listen_socket->Open(error)) { + try { + listen_socket->Open(); + } catch (const std::runtime_error &e) { delete listen_socket; - return false; + throw; } listen_port = port; - return true; } void listen_global_finish(void) diff --git a/src/Listen.hxx b/src/Listen.hxx index e6b17e4f7..a581227a4 100644 --- a/src/Listen.hxx +++ b/src/Listen.hxx @@ -21,13 +21,12 @@ #define MPD_LISTEN_HXX class EventLoop; -class Error; struct Partition; extern int listen_port; -bool -listen_global_init(EventLoop &loop, Partition &partition, Error &error); +void +listen_global_init(EventLoop &loop, Partition &partition); void listen_global_finish(); diff --git a/src/Main.cxx b/src/Main.cxx index 8602992ee..1c3a3bcba 100644 --- a/src/Main.cxx +++ b/src/Main.cxx @@ -465,11 +465,7 @@ try { initialize_decoder_and_player(); - if (!listen_global_init(instance->event_loop, *instance->partition, - error)) { - LogError(error); - return EXIT_FAILURE; - } + listen_global_init(instance->event_loop, *instance->partition); #ifdef ENABLE_DAEMON daemonize_set_user(); diff --git a/src/event/ServerSocket.cxx b/src/event/ServerSocket.cxx index 3f34a5736..c4b03ca65 100644 --- a/src/event/ServerSocket.cxx +++ b/src/event/ServerSocket.cxx @@ -30,8 +30,9 @@ #include "system/fd_util.h" #include "fs/AllocatedPath.hxx" #include "fs/FileSystem.hxx" -#include "util/Error.hxx" +#include "util/RuntimeError.hxx" #include "util/Domain.hxx" +#include "util/ScopeExit.hxx" #include "Log.hxx" #include @@ -96,7 +97,7 @@ public: } #endif - bool Open(Error &error); + void Open(); using SocketMonitor::IsDefined; using SocketMonitor::Close; @@ -179,17 +180,14 @@ OneServerSocket::OnSocketReady(gcc_unused unsigned flags) return true; } -inline bool -OneServerSocket::Open(Error &error) +inline void +OneServerSocket::Open() { assert(!IsDefined()); int _fd = socket_bind_listen(address.GetFamily(), SOCK_STREAM, 0, - address, 5, - error); - if (_fd < 0) - return false; + address, 5); #ifdef HAVE_UN /* allow everybody to connect */ @@ -201,8 +199,6 @@ OneServerSocket::Open(Error &error) /* register in the EventLoop */ SetFD(_fd); - - return true; } ServerSocket::ServerSocket(EventLoop &_loop) @@ -212,11 +208,11 @@ ServerSocket::ServerSocket(EventLoop &_loop) declaration */ ServerSocket::~ServerSocket() {} -bool -ServerSocket::Open(Error &error) +void +ServerSocket::Open() { OneServerSocket *good = nullptr, *bad = nullptr; - Error last_error; + std::exception_ptr last_error; for (auto &i : sockets) { assert(i.GetSerial() > 0); @@ -224,30 +220,32 @@ ServerSocket::Open(Error &error) if (bad != nullptr && i.GetSerial() != bad->GetSerial()) { Close(); - error = std::move(last_error); - return false; + std::rethrow_exception(last_error); } - Error error2; - if (!i.Open(error2)) { + try { + i.Open(); + } catch (const std::runtime_error &e) { if (good != nullptr && good->GetSerial() == i.GetSerial()) { const auto address_string = i.ToString(); const auto good_string = good->ToString(); - FormatWarning(server_socket_domain, - "bind to '%s' failed: %s " - "(continuing anyway, because " - "binding to '%s' succeeded)", - address_string.c_str(), - error2.GetMessage(), - good_string.c_str()); + FormatError(e, + "bind to '%s' failed " + "(continuing anyway, because " + "binding to '%s' succeeded)", + address_string.c_str(), + good_string.c_str()); } else if (bad == nullptr) { bad = &i; const auto address_string = i.ToString(); - error2.FormatPrefix("Failed to bind to '%s': ", - address_string.c_str()); - last_error = std::move(error2); + try { + std::throw_with_nested(FormatRuntimeError("Failed to bind to '%s'", + address_string.c_str())); + } catch (...) { + last_error = std::current_exception(); + } } continue; @@ -260,17 +258,14 @@ ServerSocket::Open(Error &error) if (bad != nullptr) { bad = nullptr; - last_error.Clear(); + last_error = nullptr; } } if (bad != nullptr) { Close(); - error = std::move(last_error); - return false; + std::rethrow_exception(last_error); } - - return true; } void @@ -299,26 +294,21 @@ ServerSocket::AddAddress(AllocatedSocketAddress &&address) return sockets.back(); } -bool -ServerSocket::AddFD(int fd, Error &error) +void +ServerSocket::AddFD(int fd) { assert(fd >= 0); StaticSocketAddress address; socklen_t address_length = sizeof(address); if (getsockname(fd, address.GetAddress(), - &address_length) < 0) { - SetSocketError(error); - error.AddPrefix("Failed to get socket address: "); - return false; - } + &address_length) < 0) + throw MakeSocketError("Failed to get socket address"); address.SetSize(address_length); OneServerSocket &s = AddAddress(address); s.SetFD(fd); - - return true; } #ifdef HAVE_TCP @@ -367,14 +357,12 @@ SupportsIPv6() #endif /* HAVE_TCP */ -bool -ServerSocket::AddPort(unsigned port, Error &error) +void +ServerSocket::AddPort(unsigned port) { #ifdef HAVE_TCP - if (port == 0 || port > 0xffff) { - error.Set(server_socket_domain, "Invalid TCP port"); - return false; - } + if (port == 0 || port > 0xffff) + throw std::runtime_error("Invalid TCP port"); #ifdef HAVE_IPV6 if (SupportsIPv6()) @@ -383,49 +371,37 @@ ServerSocket::AddPort(unsigned port, Error &error) AddPortIPv4(port); ++next_serial; - - return true; #else /* HAVE_TCP */ (void)port; - error.Set(server_socket_domain, "TCP support is disabled"); - return false; + throw std::runtime_error("TCP support is disabled"); #endif /* HAVE_TCP */ } -bool -ServerSocket::AddHost(const char *hostname, unsigned port, Error &error) +void +ServerSocket::AddHost(const char *hostname, unsigned port) { #ifdef HAVE_TCP struct addrinfo *ai = resolve_host_port(hostname, port, - AI_PASSIVE, SOCK_STREAM, - error); - if (ai == nullptr) - return false; + AI_PASSIVE, SOCK_STREAM); + AtScopeExit(ai) { freeaddrinfo(ai); }; for (const struct addrinfo *i = ai; i != nullptr; i = i->ai_next) AddAddress(SocketAddress(i->ai_addr, i->ai_addrlen)); - freeaddrinfo(ai); - ++next_serial; - - return true; #else /* HAVE_TCP */ (void)hostname; (void)port; - error.Set(server_socket_domain, "TCP support is disabled"); - return false; + throw std::runtime_error("TCP support is disabled"); #endif /* HAVE_TCP */ } -bool -ServerSocket::AddPath(AllocatedPath &&path, Error &error) +void +ServerSocket::AddPath(AllocatedPath &&path) { #ifdef HAVE_UN - (void)error; - unlink(path.c_str()); AllocatedSocketAddress address; @@ -433,14 +409,10 @@ ServerSocket::AddPath(AllocatedPath &&path, Error &error) OneServerSocket &s = AddAddress(std::move(address)); s.SetPath(std::move(path)); - - return true; #else /* !HAVE_UN */ (void)path; - error.Set(server_socket_domain, - "UNIX domain socket support is disabled"); - return false; + throw std::runtime_error("UNIX domain socket support is disabled"); #endif /* !HAVE_UN */ } diff --git a/src/event/ServerSocket.hxx b/src/event/ServerSocket.hxx index 252dad66f..f6e8b1152 100644 --- a/src/event/ServerSocket.hxx +++ b/src/event/ServerSocket.hxx @@ -25,7 +25,6 @@ class SocketAddress; class AllocatedSocketAddress; class EventLoop; -class Error; class AllocatedPath; class OneServerSocket; @@ -71,40 +70,49 @@ public: /** * Add a listener on a port on all interfaces. * + * Throws #std::runtime_error on error. + * * @param port the TCP port * @param error location to store the error occurring - * @return true on success */ - bool AddPort(unsigned port, Error &error); + void AddPort(unsigned port); /** * Resolves a host name, and adds listeners on all addresses in the * result set. * + * Throws #std::runtime_error on error. + * * @param hostname the host name to be resolved * @param port the TCP port * @param error location to store the error occurring - * @return true on success */ - bool AddHost(const char *hostname, unsigned port, Error &error); + void AddHost(const char *hostname, unsigned port); /** * Add a listener on a Unix domain socket. * + * Throws #std::runtime_error on error. + * * @param path the absolute socket path * @param error location to store the error occurring - * @return true on success */ - bool AddPath(AllocatedPath &&path, Error &error); + void AddPath(AllocatedPath &&path); /** * Add a socket descriptor that is accepting connections. After this * has been called, don't call server_socket_open(), because the * socket is already open. + * + * Throws #std::runtime_error on error. */ - bool AddFD(int fd, Error &error); + void AddFD(int fd); + + /** + * Throws #std::runtime_error on error. + */ + void Open(); - bool Open(Error &error); void Close(); protected: diff --git a/src/net/Resolver.cxx b/src/net/Resolver.cxx index aabf00198..66a55cce6 100644 --- a/src/net/Resolver.cxx +++ b/src/net/Resolver.cxx @@ -19,8 +19,7 @@ #include "config.h" #include "Resolver.hxx" -#include "util/Error.hxx" -#include "util/Domain.hxx" +#include "util/RuntimeError.hxx" #include @@ -35,12 +34,9 @@ #include #include -const Domain resolver_domain("resolver"); - struct addrinfo * resolve_host_port(const char *host_port, unsigned default_port, - int flags, int socktype, - Error &error) + int flags, int socktype) { std::string p(host_port); const char *host = p.c_str(), *port = nullptr; @@ -87,12 +83,9 @@ resolve_host_port(const char *host_port, unsigned default_port, struct addrinfo *ai; int ret = getaddrinfo(host, port, &hints, &ai); - if (ret != 0) { - error.Format(resolver_domain, ret, - "Failed to look up '%s': %s", - host_port, gai_strerror(ret)); - return nullptr; - } + if (ret != 0) + throw FormatRuntimeError("Failed to look up '%s': %s", + host_port, gai_strerror(ret)); return ai; } diff --git a/src/net/Resolver.hxx b/src/net/Resolver.hxx index 2683a0d43..5f3577b90 100644 --- a/src/net/Resolver.hxx +++ b/src/net/Resolver.hxx @@ -24,24 +24,21 @@ #include "Compiler.h" struct addrinfo; -class Error; -class Domain; - -extern const Domain resolver_domain; /** * Resolve a specification in the form "host", "host:port", * "[host]:port". This is a convenience wrapper for getaddrinfo(). * + * Throws #std::runtime_error on error. + * * @param default_port a default port number that will be used if none * is given in the string (if applicable); pass 0 to go without a * default * @return an #addrinfo linked list that must be freed with - * freeaddrinfo(), or NULL on error + * freeaddrinfo() */ addrinfo * resolve_host_port(const char *host_port, unsigned default_port, - int flags, int socktype, - Error &error); + int flags, int socktype); #endif diff --git a/src/net/SocketError.hxx b/src/net/SocketError.hxx index 1e04487f8..0effe6992 100644 --- a/src/net/SocketError.hxx +++ b/src/net/SocketError.hxx @@ -21,6 +21,7 @@ #define MPD_SOCKET_ERROR_HXX #include "Compiler.h" +#include "system/Error.hxx" #include "util/Error.hxx" // IWYU pragma: export #ifdef WIN32 @@ -136,4 +137,22 @@ NewSocketError() return NewSocketError(GetSocketError()); } +gcc_const +static inline std::system_error +MakeSocketError(socket_error_t code, const char *msg) +{ +#ifdef WIN32 + return MakeLastError(code, msg); +#else + return MakeErrno(code, msg); +#endif +} + +gcc_pure +static inline std::system_error +MakeSocketError(const char *msg) +{ + return MakeSocketError(GetSocketError(), msg); +} + #endif diff --git a/src/net/SocketUtil.cxx b/src/net/SocketUtil.cxx index 0f89cac1f..a35bd8639 100644 --- a/src/net/SocketUtil.cxx +++ b/src/net/SocketUtil.cxx @@ -26,41 +26,35 @@ int socket_bind_listen(int domain, int type, int protocol, SocketAddress address, - int backlog, - Error &error) + int backlog) { int fd, ret; const int reuse = 1; fd = socket_cloexec_nonblock(domain, type, protocol); - if (fd < 0) { - SetSocketError(error); - error.AddPrefix("Failed to create socket: "); - return -1; - } + if (fd < 0) + throw MakeSocketError("Failed to create socket"); ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *) &reuse, sizeof(reuse)); if (ret < 0) { - SetSocketError(error); - error.AddPrefix("setsockopt() failed: "); + auto error = GetSocketError(); close_socket(fd); - return -1; + throw MakeSocketError(error, "setsockopt() failed"); } ret = bind(fd, address.GetAddress(), address.GetSize()); if (ret < 0) { - SetSocketError(error); + auto error = GetSocketError(); close_socket(fd); - return -1; + throw MakeSocketError(error, "Failed to bind socket"); } ret = listen(fd, backlog); if (ret < 0) { - SetSocketError(error); - error.AddPrefix("listen() failed: "); + auto error = GetSocketError(); close_socket(fd); - return -1; + throw MakeSocketError(error, "Failed to listen on socket"); } #if defined(HAVE_STRUCT_UCRED) && defined(SO_PASSCRED) diff --git a/src/net/SocketUtil.hxx b/src/net/SocketUtil.hxx index 1111de8ef..2e5cfaafe 100644 --- a/src/net/SocketUtil.hxx +++ b/src/net/SocketUtil.hxx @@ -27,12 +27,13 @@ #define MPD_SOCKET_UTIL_HXX class SocketAddress; -class Error; /** * Creates a socket listening on the specified address. This is a * shortcut for socket(), bind() and listen(). * + * Throws #std::system_error on error. + * * @param domain the socket domain, e.g. PF_INET6 * @param type the socket type, e.g. SOCK_STREAM * @param protocol the protocol, usually 0 to let the kernel choose @@ -40,13 +41,12 @@ class Error; * @param backlog the backlog parameter for the listen() system call * @param error location to store the error occurring, or NULL to * ignore errors - * @return the socket file descriptor or -1 on error + * @return the socket file descriptor */ int socket_bind_listen(int domain, int type, int protocol, SocketAddress address, - int backlog, - Error &error); + int backlog); int socket_keepalive(int fd); diff --git a/src/output/plugins/httpd/HttpdInternal.hxx b/src/output/plugins/httpd/HttpdInternal.hxx index f554c7cf6..46ec3e13b 100644 --- a/src/output/plugins/httpd/HttpdInternal.hxx +++ b/src/output/plugins/httpd/HttpdInternal.hxx @@ -177,7 +177,7 @@ public: return &base; } - bool Bind(Error &error); + void Bind(); void Unbind(); /** diff --git a/src/output/plugins/httpd/HttpdOutputPlugin.cxx b/src/output/plugins/httpd/HttpdOutputPlugin.cxx index 8438b736d..15115bb2b 100644 --- a/src/output/plugins/httpd/HttpdOutputPlugin.cxx +++ b/src/output/plugins/httpd/HttpdOutputPlugin.cxx @@ -66,16 +66,14 @@ HttpdOutput::~HttpdOutput() delete prepared_encoder; } -inline bool -HttpdOutput::Bind(Error &error) +inline void +HttpdOutput::Bind() { open = false; - bool result = false; - BlockingCall(GetEventLoop(), [this, &error, &result](){ - result = ServerSocket::Open(error); + BlockingCall(GetEventLoop(), [this](){ + ServerSocket::Open(); }); - return result; } inline void @@ -112,12 +110,10 @@ HttpdOutput::Configure(const ConfigBlock &block, Error &error) /* set up bind_to_address */ const char *bind_to_address = block.GetBlockValue("bind_to_address"); - bool success = bind_to_address != nullptr && - strcmp(bind_to_address, "any") != 0 - ? AddHost(bind_to_address, port, error) - : AddPort(port, error); - if (!success) - return false; + if (bind_to_address != nullptr && strcmp(bind_to_address, "any") != 0) + AddHost(bind_to_address, port); + else + AddPort(port); /* initialize encoder */ @@ -144,11 +140,16 @@ httpd_output_init(const ConfigBlock &block, Error &error) { HttpdOutput *httpd = new HttpdOutput(io_thread_get()); - AudioOutput *result = httpd->InitAndConfigure(block, error); - if (result == nullptr) - delete httpd; + try { + AudioOutput *result = httpd->InitAndConfigure(block, error); + if (result == nullptr) + delete httpd; - return result; + return result; + } catch (const std::runtime_error &e) { + delete httpd; + throw; + } } static void @@ -271,11 +272,12 @@ HttpdOutput::ReadPage() } static bool -httpd_output_enable(AudioOutput *ao, Error &error) +httpd_output_enable(AudioOutput *ao, gcc_unused Error &error) { HttpdOutput *httpd = HttpdOutput::Cast(ao); - return httpd->Bind(error); + httpd->Bind(); + return true; } static void diff --git a/test/run_resolver.cxx b/test/run_resolver.cxx index ac51c0ea1..a7ff0d3ad 100644 --- a/test/run_resolver.cxx +++ b/test/run_resolver.cxx @@ -21,9 +21,10 @@ #include "net/Resolver.hxx" #include "net/ToString.hxx" #include "net/SocketAddress.hxx" -#include "util/Error.hxx" #include "Log.hxx" +#include + #ifdef WIN32 #include #include @@ -36,20 +37,14 @@ #include int main(int argc, char **argv) -{ +try { if (argc != 2) { fprintf(stderr, "Usage: run_resolver HOST\n"); return EXIT_FAILURE; } - Error error; struct addrinfo *ai = - resolve_host_port(argv[1], 80, AI_PASSIVE, SOCK_STREAM, - error); - if (ai == NULL) { - LogError(error); - return EXIT_FAILURE; - } + resolve_host_port(argv[1], 80, AI_PASSIVE, SOCK_STREAM); for (const struct addrinfo *i = ai; i != NULL; i = i->ai_next) { const auto s = ToString({i->ai_addr, i->ai_addrlen}); @@ -58,4 +53,7 @@ int main(int argc, char **argv) freeaddrinfo(ai); return EXIT_SUCCESS; +} catch (const std::runtime_error &e) { + LogError(e); + return EXIT_FAILURE; }