From ba6bc50bb13698b40ea1b0caa47fc7f577c9a928 Mon Sep 17 00:00:00 2001 From: rubidium42 Date: Fri, 30 Apr 2021 15:39:46 +0200 Subject: [PATCH 1/3] Codechange: rename NetworkError to ShowNetworkError --- src/network/core/tcp_listen.h | 2 +- src/network/network.cpp | 4 ++-- src/network/network_client.cpp | 2 +- src/network/network_internal.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/network/core/tcp_listen.h b/src/network/core/tcp_listen.h index 168f49f9470ff..2ceea20aa99e9 100644 --- a/src/network/core/tcp_listen.h +++ b/src/network/core/tcp_listen.h @@ -151,7 +151,7 @@ class TCPListenHandler { if (sockets.size() == 0) { DEBUG(net, 0, "[server] could not start network: could not create listening socket"); - NetworkError(STR_NETWORK_ERROR_SERVER_START); + ShowNetworkError(STR_NETWORK_ERROR_SERVER_START); return false; } diff --git a/src/network/network.cpp b/src/network/network.cpp index c31a67487d25e..9937242241ec2 100644 --- a/src/network/network.cpp +++ b/src/network/network.cpp @@ -277,7 +277,7 @@ uint NetworkCalculateLag(const NetworkClientSocket *cs) /* There was a non-recoverable error, drop back to the main menu with a nice * error */ -void NetworkError(StringID error_string) +void ShowNetworkError(StringID error_string) { _switch_mode = SM_MENU; ShowErrorMessage(error_string, INVALID_STRING_ID, WL_CRITICAL); @@ -701,7 +701,7 @@ class TCPClientConnecter : TCPConnecter { void OnFailure() override { - NetworkError(STR_NETWORK_ERROR_NOCONNECTION); + ShowNetworkError(STR_NETWORK_ERROR_NOCONNECTION); } void OnConnect(SOCKET s) override diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp index f73c8be528c78..85542b8a8a2a8 100644 --- a/src/network/network_client.cpp +++ b/src/network/network_client.cpp @@ -284,7 +284,7 @@ void ClientNetworkGameSocketHandler::ClientError(NetworkRecvStatus res) #else if (_sync_seed_1 != _random.state[0]) { #endif - NetworkError(STR_NETWORK_ERROR_DESYNC); + ShowNetworkError(STR_NETWORK_ERROR_DESYNC); DEBUG(desync, 1, "sync_err: %08x; %02x", _date, _date_fract); DEBUG(net, 0, "Sync error detected!"); my_client->ClientError(NETWORK_RECV_STATUS_DESYNC); diff --git a/src/network/network_internal.h b/src/network/network_internal.h index 683c954e8256a..ad6317e6c8c16 100644 --- a/src/network/network_internal.h +++ b/src/network/network_internal.h @@ -112,7 +112,7 @@ void NetworkExecuteLocalCommandQueue(); void NetworkFreeLocalCommandQueue(); void NetworkSyncCommandQueue(NetworkClientSocket *cs); -void NetworkError(StringID error_string); +void ShowNetworkError(StringID error_string); void NetworkTextMessage(NetworkAction action, TextColour colour, bool self_send, const char *name, const char *str = "", int64 data = 0); uint NetworkCalculateLag(const NetworkClientSocket *cs); StringID GetNetworkErrorMsg(NetworkErrorCode err); From f51258d9dede0997ce3fa3d1e7812d63a520e69f Mon Sep 17 00:00:00 2001 From: rubidium42 Date: Fri, 30 Apr 2021 15:38:22 +0200 Subject: [PATCH 2/3] Codechange: encapsulate network error handling --- src/network/core/CMakeLists.txt | 1 + src/network/core/address.cpp | 24 +++--- src/network/core/core.cpp | 18 ---- src/network/core/os_abstraction.cpp | 125 ++++++++++++++++++++++++++++ src/network/core/os_abstraction.h | 52 +++++------- src/network/core/tcp.cpp | 22 ++--- src/network/core/tcp_http.cpp | 8 +- src/network/core/tcp_listen.h | 4 +- src/network/core/udp.cpp | 4 +- src/safeguards.h | 8 +- 10 files changed, 183 insertions(+), 83 deletions(-) create mode 100644 src/network/core/os_abstraction.cpp diff --git a/src/network/core/CMakeLists.txt b/src/network/core/CMakeLists.txt index 37cc3e1954442..bf713be99ce95 100644 --- a/src/network/core/CMakeLists.txt +++ b/src/network/core/CMakeLists.txt @@ -8,6 +8,7 @@ add_files( game_info.h host.cpp host.h + os_abstraction.cpp os_abstraction.h packet.cpp packet.h diff --git a/src/network/core/address.cpp b/src/network/core/address.cpp index e91751c33f08a..d3e373ef7684b 100644 --- a/src/network/core/address.cpp +++ b/src/network/core/address.cpp @@ -317,7 +317,7 @@ static SOCKET ConnectLoopProc(addrinfo *runp) SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol); if (sock == INVALID_SOCKET) { - DEBUG(net, 1, "[%s] could not create %s socket: %s", type, family, NetworkGetLastErrorString()); + DEBUG(net, 1, "[%s] could not create %s socket: %s", type, family, NetworkError::GetLast().AsString()); return INVALID_SOCKET; } @@ -326,8 +326,8 @@ static SOCKET ConnectLoopProc(addrinfo *runp) if (!SetNonBlocking(sock)) DEBUG(net, 0, "[%s] setting non-blocking mode failed", type); int err = connect(sock, runp->ai_addr, (int)runp->ai_addrlen); - if (err != 0 && NetworkGetLastError() != EINPROGRESS) { - DEBUG(net, 1, "[%s] could not connect to %s over %s: %s", type, address.c_str(), family, NetworkGetLastErrorString()); + if (err != 0 && !NetworkError::GetLast().IsConnectInProgress()) { + DEBUG(net, 1, "[%s] could not connect to %s over %s: %s", type, address.c_str(), family, NetworkError::GetLast().AsString()); closesocket(sock); return INVALID_SOCKET; } @@ -343,7 +343,7 @@ static SOCKET ConnectLoopProc(addrinfo *runp) tv.tv_sec = DEFAULT_CONNECT_TIMEOUT_SECONDS; int n = select(FD_SETSIZE, NULL, &write_fd, NULL, &tv); if (n < 0) { - DEBUG(net, 1, "[%s] could not connect to %s: %s", type, address.c_str(), NetworkGetLastErrorString()); + DEBUG(net, 1, "[%s] could not connect to %s: %s", type, address.c_str(), NetworkError::GetLast().AsString()); closesocket(sock); return INVALID_SOCKET; } @@ -356,9 +356,9 @@ static SOCKET ConnectLoopProc(addrinfo *runp) } /* Retrieve last error, if any, on the socket. */ - err = GetSocketError(sock); - if (err != 0) { - DEBUG(net, 1, "[%s] could not connect to %s: %s", type, address.c_str(), NetworkGetErrorString(err)); + NetworkError socket_error = GetSocketError(sock); + if (socket_error.HasError()) { + DEBUG(net, 1, "[%s] could not connect to %s: %s", type, address.c_str(), socket_error.AsString()); closesocket(sock); return INVALID_SOCKET; } @@ -393,7 +393,7 @@ static SOCKET ListenLoopProc(addrinfo *runp) SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol); if (sock == INVALID_SOCKET) { - DEBUG(net, 0, "[%s] could not create %s socket on port %s: %s", type, family, address.c_str(), NetworkGetLastErrorString()); + DEBUG(net, 0, "[%s] could not create %s socket on port %s: %s", type, family, address.c_str(), NetworkError::GetLast().AsString()); return INVALID_SOCKET; } @@ -404,24 +404,24 @@ static SOCKET ListenLoopProc(addrinfo *runp) int on = 1; /* The (const char*) cast is needed for windows!! */ if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (const char*)&on, sizeof(on)) == -1) { - DEBUG(net, 3, "[%s] could not set reusable %s sockets for port %s: %s", type, family, address.c_str(), NetworkGetLastErrorString()); + DEBUG(net, 3, "[%s] could not set reusable %s sockets for port %s: %s", type, family, address.c_str(), NetworkError::GetLast().AsString()); } #ifndef __OS2__ if (runp->ai_family == AF_INET6 && setsockopt(sock, IPPROTO_IPV6, IPV6_V6ONLY, (const char*)&on, sizeof(on)) == -1) { - DEBUG(net, 3, "[%s] could not disable IPv4 over IPv6 on port %s: %s", type, address.c_str(), NetworkGetLastErrorString()); + DEBUG(net, 3, "[%s] could not disable IPv4 over IPv6 on port %s: %s", type, address.c_str(), NetworkError::GetLast().AsString()); } #endif if (bind(sock, runp->ai_addr, (int)runp->ai_addrlen) != 0) { - DEBUG(net, 1, "[%s] could not bind on %s port %s: %s", type, family, address.c_str(), NetworkGetLastErrorString()); + DEBUG(net, 1, "[%s] could not bind on %s port %s: %s", type, family, address.c_str(), NetworkError::GetLast().AsString()); closesocket(sock); return INVALID_SOCKET; } if (runp->ai_socktype != SOCK_DGRAM && listen(sock, 1) != 0) { - DEBUG(net, 1, "[%s] could not listen at %s port %s: %s", type, family, address.c_str(), NetworkGetLastErrorString()); + DEBUG(net, 1, "[%s] could not listen at %s port %s: %s", type, family, address.c_str(), NetworkError::GetLast().AsString()); closesocket(sock); return INVALID_SOCKET; } diff --git a/src/network/core/core.cpp b/src/network/core/core.cpp index 5c12cb2242be0..563deae96315e 100644 --- a/src/network/core/core.cpp +++ b/src/network/core/core.cpp @@ -13,7 +13,6 @@ #include "../../debug.h" #include "os_abstraction.h" #include "packet.h" -#include "../../string_func.h" #include "../../safeguards.h" @@ -48,20 +47,3 @@ void NetworkCoreShutdown() WSACleanup(); #endif } - -#if defined(_WIN32) -/** - * Return the string representation of the given error from the OS's network functions. - * @param error The error number (from \c NetworkGetLastError()). - * @return The error message, potentially an empty string but never \c nullptr. - */ -const char *NetworkGetErrorString(int error) -{ - static char buffer[512]; - if (FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, NULL, error, - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), buffer, sizeof(buffer), NULL) == 0) { - seprintf(buffer, lastof(buffer), "Unknown error %d", error); - } - return buffer; -} -#endif /* defined(_WIN32) */ diff --git a/src/network/core/os_abstraction.cpp b/src/network/core/os_abstraction.cpp new file mode 100644 index 0000000000000..75f2224eb0dfa --- /dev/null +++ b/src/network/core/os_abstraction.cpp @@ -0,0 +1,125 @@ +/* + * This file is part of OpenTTD. + * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. + * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . + */ + +/** + * @file os_abstraction.cpp OS specific implementations of functions of the OS abstraction layer for network stuff. + * + * The general idea is to have simple abstracting functions for things that + * require different implementations for different environments. + * In here the functions, and their documentation, are defined only once + * and the implementation contains the #ifdefs to change the implementation. + * Since Windows is usually different that is usually the first case, after + * that the behaviour is usually Unix/BSD-like with occasional variation. + */ + +#include "stdafx.h" +#include "os_abstraction.h" +#include "../../string_func.h" +#include + +#include "../../safeguards.h" + +/** + * Construct the network error with the given error code. + * @param error The error code. + */ +NetworkError::NetworkError(int error) : error(error) +{ +} + +/** + * Check whether this error describes that the operation would block. + * @return True iff the operation would block. + */ +bool NetworkError::WouldBlock() const +{ +#if defined(_WIN32) + return this->error == WSAEWOULDBLOCK; +#else + /* Usually EWOULDBLOCK and EAGAIN are the same, but sometimes they are not + * and the POSIX.1 specification states that either should be checked. */ + return this->error == EWOULDBLOCK || this->error == EAGAIN; +#endif +} + +/** + * Check whether this error describes a connection reset. + * @return True iff the connection is reset. + */ +bool NetworkError::IsConnectionReset() const +{ +#if defined(_WIN32) + return this->error == WSAECONNRESET; +#else + return this->error == ECONNRESET; +#endif +} + +/** + * Check whether this error describes a connect is in progress. + * @return True iff the connect is already in progress. + */ +bool NetworkError::IsConnectInProgress() const +{ +#if defined(_WIN32) + return this->error == WSAEWOULDBLOCK; +#else + return this->error == EINPROGRESS; +#endif +} + +/** + * Get the string representation of the error message. + * @return The string representation that will get overwritten by next calls. + */ +const char *NetworkError::AsString() const +{ + if (this->message.empty()) { +#if defined(_WIN32) + char buffer[512]; + if (FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, NULL, this->error, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), buffer, sizeof(buffer), NULL) == 0) { + seprintf(buffer, lastof(buffer), "Unknown error %d", this->error); + } + this->message.assign(buffer); +#else + /* Make strerror thread safe by locking access to it. There is a thread safe strerror_r, however + * the non-POSIX variant is available due to defining _GNU_SOURCE meaning it is not portable. + * The problem with the non-POSIX variant is that it does not necessarily fill the buffer with + * the error message but can also return a pointer to a static bit of memory, whereas the POSIX + * variant always fills the buffer. This makes the behaviour too erratic to work with. */ + static std::mutex mutex; + std::lock_guard guard(mutex); + this->message.assign(strerror(this->error)); +#endif + } + return this->message.c_str(); +} + +/** + * Check whether an error was actually set. + * @return True iff an error was set. + */ +bool NetworkError::HasError() const +{ + return this->error != 0; +} + +/** + * Get the last network error. + * @return The network error. + */ +/* static */ NetworkError NetworkError::GetLast() +{ +#if defined(_WIN32) + return NetworkError(WSAGetLastError()); +#elif defined(__OS2__) + return NetworkError(sock_errno()); +#else + return NetworkError(errno); +#endif +} diff --git a/src/network/core/os_abstraction.h b/src/network/core/os_abstraction.h index 9bd0e321f711b..e444bc78b4c1c 100644 --- a/src/network/core/os_abstraction.h +++ b/src/network/core/os_abstraction.h @@ -14,6 +14,26 @@ #ifndef NETWORK_CORE_OS_ABSTRACTION_H #define NETWORK_CORE_OS_ABSTRACTION_H +/** + * Abstraction of a network error where all implementation details of the + * error codes are encapsulated in this class and the abstraction layer. + */ +class NetworkError { +private: + int error; ///< The underlying error number from errno or WSAGetLastError. + mutable std::string message; ///< The string representation of the error (set on first call to #AsString). +public: + NetworkError(int error); + + bool HasError() const; + bool WouldBlock() const; + bool IsConnectionReset() const; + bool IsConnectInProgress() const; + const char *AsString() const; + + static NetworkError GetLast(); +}; + /* Include standard stuff per OS */ /* Windows stuff */ @@ -23,21 +43,6 @@ #include #include -/** - * Get the last error code from any of the OS's network functions. - * What it returns and when it is reset, is implementation defined. - * @return The last error code. - */ -#define NetworkGetLastError() WSAGetLastError() -#undef EWOULDBLOCK -#define EWOULDBLOCK WSAEWOULDBLOCK -#undef ECONNRESET -#define ECONNRESET WSAECONNRESET -#undef EINPROGRESS -#define EINPROGRESS WSAEWOULDBLOCK - -const char *NetworkGetErrorString(int error); - /* Windows has some different names for some types */ typedef unsigned long in_addr_t; @@ -63,8 +68,6 @@ typedef unsigned long in_addr_t; # define INVALID_SOCKET -1 # define ioctlsocket ioctl # define closesocket close -# define NetworkGetLastError() (errno) -# define NetworkGetErrorString(error) (strerror(error)) /* Need this for FIONREAD on solaris */ # define BSD_COMP @@ -114,8 +117,6 @@ typedef unsigned long in_addr_t; # define INVALID_SOCKET -1 # define ioctlsocket ioctl # define closesocket close -# define NetworkGetLastError() (sock_errno()) -# define NetworkGetErrorString(error) (strerror(error)) /* Includes needed for OS/2 systems */ # include @@ -187,15 +188,6 @@ static inline socklen_t FixAddrLenForEmscripten(struct sockaddr_storage &address } #endif -/** - * Return the string representation of the last error from the OS's network functions. - * @return The error message, potentially an empty string but never \c nullptr. - */ -static inline const char *NetworkGetLastErrorString() -{ - return NetworkGetErrorString(NetworkGetLastError()); -} - /** * Try to set the socket into non-blocking mode. * @param d The socket to set the non-blocking more for. @@ -237,13 +229,13 @@ static inline bool SetNoDelay(SOCKET d) * @param d The socket to get the error from. * @return The errno on the socket. */ -static inline int GetSocketError(SOCKET d) +static inline NetworkError GetSocketError(SOCKET d) { int err; socklen_t len = sizeof(err); getsockopt(d, SOL_SOCKET, SO_ERROR, (char *)&err, &len); - return err; + return NetworkError(err); } /* Make sure these structures have the size we expect them to be */ diff --git a/src/network/core/tcp.cpp b/src/network/core/tcp.cpp index f23b202c8b891..842e1a89b9676 100644 --- a/src/network/core/tcp.cpp +++ b/src/network/core/tcp.cpp @@ -86,11 +86,11 @@ SendPacketsState NetworkTCPSocketHandler::SendPackets(bool closing_down) while ((p = this->packet_queue) != nullptr) { res = p->TransferOut(send, this->sock, 0); if (res == -1) { - int err = NetworkGetLastError(); - if (err != EWOULDBLOCK) { + NetworkError err = NetworkError::GetLast(); + if (!err.WouldBlock()) { /* Something went wrong.. close client! */ if (!closing_down) { - DEBUG(net, 0, "send failed with error %s", NetworkGetErrorString(err)); + DEBUG(net, 0, "send failed with error %s", err.AsString()); this->CloseConnection(); } return SPS_CLOSED; @@ -136,10 +136,10 @@ Packet *NetworkTCPSocketHandler::ReceivePacket() while (p->RemainingBytesToTransfer() != 0) { res = p->TransferIn(recv, this->sock, 0); if (res == -1) { - int err = NetworkGetLastError(); - if (err != EWOULDBLOCK) { - /* Something went wrong... (ECONNRESET is connection reset by peer) */ - if (err != ECONNRESET) DEBUG(net, 0, "recv failed with error %s", NetworkGetErrorString(err)); + NetworkError err = NetworkError::GetLast(); + if (!err.WouldBlock()) { + /* Something went wrong... */ + if (!err.IsConnectionReset()) DEBUG(net, 0, "recv failed with error %s", err.AsString()); this->CloseConnection(); return nullptr; } @@ -164,10 +164,10 @@ Packet *NetworkTCPSocketHandler::ReceivePacket() while (p->RemainingBytesToTransfer() != 0) { res = p->TransferIn(recv, this->sock, 0); if (res == -1) { - int err = NetworkGetLastError(); - if (err != EWOULDBLOCK) { - /* Something went wrong... (ECONNRESET is connection reset by peer) */ - if (err != ECONNRESET) DEBUG(net, 0, "recv failed with error %s", NetworkGetErrorString(err)); + NetworkError err = NetworkError::GetLast(); + if (!err.WouldBlock()) { + /* Something went wrong... */ + if (!err.IsConnectionReset()) DEBUG(net, 0, "recv failed with error %s", err.AsString()); this->CloseConnection(); return nullptr; } diff --git a/src/network/core/tcp_http.cpp b/src/network/core/tcp_http.cpp index e0c269fafb4b9..4f29df1912ae6 100644 --- a/src/network/core/tcp_http.cpp +++ b/src/network/core/tcp_http.cpp @@ -225,10 +225,10 @@ int NetworkHTTPSocketHandler::Receive() for (;;) { ssize_t res = recv(this->sock, (char *)this->recv_buffer + this->recv_pos, lengthof(this->recv_buffer) - this->recv_pos, 0); if (res == -1) { - int err = NetworkGetLastError(); - if (err != EWOULDBLOCK) { - /* Something went wrong... (ECONNRESET is connection reset by peer) */ - if (err != ECONNRESET) DEBUG(net, 0, "recv failed with error %s", NetworkGetErrorString(err)); + NetworkError err = NetworkError::GetLast(); + if (!err.WouldBlock()) { + /* Something went wrong... */ + if (!err.IsConnectionReset()) DEBUG(net, 0, "recv failed with error %s", err.AsString()); return -1; } /* Connection would block, so stop for now */ diff --git a/src/network/core/tcp_listen.h b/src/network/core/tcp_listen.h index 2ceea20aa99e9..e23ecae70748b 100644 --- a/src/network/core/tcp_listen.h +++ b/src/network/core/tcp_listen.h @@ -64,7 +64,7 @@ class TCPListenHandler { DEBUG(net, 1, "[%s] Banned ip tried to join (%s), refused", Tsocket::GetName(), entry.c_str()); if (p.TransferOut(send, s, 0) < 0) { - DEBUG(net, 0, "send failed with error %s", NetworkGetLastErrorString()); + DEBUG(net, 0, "send failed with error %s", NetworkError::GetLast().AsString()); } closesocket(s); break; @@ -81,7 +81,7 @@ class TCPListenHandler { p.PrepareToSend(); if (p.TransferOut(send, s, 0) < 0) { - DEBUG(net, 0, "send failed with error %s", NetworkGetLastErrorString()); + DEBUG(net, 0, "send failed with error %s", NetworkError::GetLast().AsString()); } closesocket(s); diff --git a/src/network/core/udp.cpp b/src/network/core/udp.cpp index ffc86d825fcf1..e7b99a53e8ca7 100644 --- a/src/network/core/udp.cpp +++ b/src/network/core/udp.cpp @@ -95,7 +95,7 @@ void NetworkUDPSocketHandler::SendPacket(Packet *p, NetworkAddress *recv, bool a /* Enable broadcast */ unsigned long val = 1; if (setsockopt(s.second, SOL_SOCKET, SO_BROADCAST, (char *) &val, sizeof(val)) < 0) { - DEBUG(net, 1, "[udp] setting broadcast failed with: %s", NetworkGetLastErrorString()); + DEBUG(net, 1, "[udp] setting broadcast failed with: %s", NetworkError::GetLast().AsString()); } } @@ -104,7 +104,7 @@ void NetworkUDPSocketHandler::SendPacket(Packet *p, NetworkAddress *recv, bool a DEBUG(net, 7, "[udp] sendto(%s)", send.GetAddressAsString().c_str()); /* Check for any errors, but ignore it otherwise */ - if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %s", send.GetAddressAsString().c_str(), NetworkGetLastErrorString()); + if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %s", send.GetAddressAsString().c_str(), NetworkError::GetLast().AsString()); if (!all) break; } diff --git a/src/safeguards.h b/src/safeguards.h index e3d6c4a3e4def..aca461175fc91 100644 --- a/src/safeguards.h +++ b/src/safeguards.h @@ -70,15 +70,15 @@ #endif #if defined(NETWORK_CORE_OS_ABSTRACTION_H) && defined(_WIN32) -/* Use NetworkGetLastError() instead of errno, or do not (indirectly) include network/core/os_abstraction.h. - * Winsock does not set errno, but one should rather call WSAGetLastError. NetworkGetLastError abstracts that away. */ +/* Use NetworkError::GetLast() instead of errno, or do not (indirectly) include network/core/os_abstraction.h. + * Winsock does not set errno, but one should rather call WSAGetLastError. NetworkError::GetLast abstracts that away. */ #ifdef errno #undef errno #endif #define errno SAFEGUARD_DO_NOT_USE_THIS_METHOD -/* Use NetworkGetLastErrorString() instead of strerror, or do not (indirectly) include network/core/os_abstraction.h. - * Winsock errors are not handled by strerror, but one should rather call FormatMessage. NetworkGetLastErrorString abstracts that away. */ +/* Use NetworkError::AsString() instead of strerror, or do not (indirectly) include network/core/os_abstraction.h. + * Winsock errors are not handled by strerror, but one should rather call FormatMessage. NetworkError::AsString abstracts that away. */ #define strerror SAFEGUARD_DO_NOT_USE_THIS_METHOD #endif /* defined(NETWORK_CORE_OS_ABSTRACTION_H) && defined(_WIN32) */ From 145d12879dfe9c1024073319c746c023451cbe00 Mon Sep 17 00:00:00 2001 From: rubidium42 Date: Fri, 30 Apr 2021 19:21:02 +0200 Subject: [PATCH 3/3] Codechange: move some OS abstraction method implementations out of the header --- src/network/core/os_abstraction.cpp | 49 ++++++++++++++++++++++++++ src/network/core/os_abstraction.h | 53 ++--------------------------- 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/src/network/core/os_abstraction.cpp b/src/network/core/os_abstraction.cpp index 75f2224eb0dfa..b2d3475d01994 100644 --- a/src/network/core/os_abstraction.cpp +++ b/src/network/core/os_abstraction.cpp @@ -123,3 +123,52 @@ bool NetworkError::HasError() const return NetworkError(errno); #endif } + + +/** + * Try to set the socket into non-blocking mode. + * @param d The socket to set the non-blocking more for. + * @return True if setting the non-blocking mode succeeded, otherwise false. + */ +bool SetNonBlocking(SOCKET d) +{ +#if defined(_WIN32) + u_long nonblocking = 1; + return ioctlsocket(d, FIONBIO, &nonblocking) == 0; +#elif defined __EMSCRIPTEN__ + return true; +#else + int nonblocking = 1; + return ioctl(d, FIONBIO, &nonblocking) == 0; +#endif +} + +/** + * Try to set the socket to not delay sending. + * @param d The socket to disable the delaying for. + * @return True if disabling the delaying succeeded, otherwise false. + */ +bool SetNoDelay(SOCKET d) +{ +#ifdef __EMSCRIPTEN__ + return true; +#else + int flags = 1; + /* The (const char*) cast is needed for windows */ + return setsockopt(d, IPPROTO_TCP, TCP_NODELAY, (const char *)&flags, sizeof(flags)) == 0; +#endif +} + +/** + * Get the error from a socket, if any. + * @param d The socket to get the error from. + * @return The errno on the socket. + */ +NetworkError GetSocketError(SOCKET d) +{ + int err; + socklen_t len = sizeof(err); + getsockopt(d, SOL_SOCKET, SO_ERROR, (char *)&err, &len); + + return NetworkError(err); +} diff --git a/src/network/core/os_abstraction.h b/src/network/core/os_abstraction.h index e444bc78b4c1c..55d733501758f 100644 --- a/src/network/core/os_abstraction.h +++ b/src/network/core/os_abstraction.h @@ -66,7 +66,6 @@ typedef unsigned long in_addr_t; # endif # define SOCKET int # define INVALID_SOCKET -1 -# define ioctlsocket ioctl # define closesocket close /* Need this for FIONREAD on solaris */ # define BSD_COMP @@ -115,7 +114,6 @@ typedef unsigned long in_addr_t; #if defined(__OS2__) # define SOCKET int # define INVALID_SOCKET -1 -# define ioctlsocket ioctl # define closesocket close /* Includes needed for OS/2 systems */ @@ -188,55 +186,10 @@ static inline socklen_t FixAddrLenForEmscripten(struct sockaddr_storage &address } #endif -/** - * Try to set the socket into non-blocking mode. - * @param d The socket to set the non-blocking more for. - * @return True if setting the non-blocking mode succeeded, otherwise false. - */ -static inline bool SetNonBlocking(SOCKET d) -{ -#ifdef __EMSCRIPTEN__ - return true; -#else -# ifdef _WIN32 - u_long nonblocking = 1; -# else - int nonblocking = 1; -# endif - return ioctlsocket(d, FIONBIO, &nonblocking) == 0; -#endif -} -/** - * Try to set the socket to not delay sending. - * @param d The socket to disable the delaying for. - * @return True if disabling the delaying succeeded, otherwise false. - */ -static inline bool SetNoDelay(SOCKET d) -{ -#ifdef __EMSCRIPTEN__ - return true; -#else - /* XXX should this be done at all? */ - int b = 1; - /* The (const char*) cast is needed for windows */ - return setsockopt(d, IPPROTO_TCP, TCP_NODELAY, (const char*)&b, sizeof(b)) == 0; -#endif -} - -/** - * Get the error from a socket, if any. - * @param d The socket to get the error from. - * @return The errno on the socket. - */ -static inline NetworkError GetSocketError(SOCKET d) -{ - int err; - socklen_t len = sizeof(err); - getsockopt(d, SOL_SOCKET, SO_ERROR, (char *)&err, &len); - - return NetworkError(err); -} +bool SetNonBlocking(SOCKET d); +bool SetNoDelay(SOCKET d); +NetworkError GetSocketError(SOCKET d); /* Make sure these structures have the size we expect them to be */ static_assert(sizeof(in_addr) == 4); ///< IPv4 addresses should be 4 bytes.