From 9e4c59a44e2a326071a030d54908a5aaf2dbbc84 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Fri, 8 Nov 2024 21:49:41 +0300 Subject: [PATCH 1/2] netplay: more strict checks for error codes from socket operations There isn't any guarantee that `getSockErr()`/`setSockErr()` won't return `0` in all corner cases where something bad happens with an underlying socket. Provide more strict checks for socket write errors and for checking the result of connection opening routines, allowing to catch error conditions even in presence of error code == 0. Signed-off-by: Pavel Solodovnikov --- lib/netplay/netsocket.cpp | 8 ++++---- lib/netplay/netsocket.h | 12 ++++++------ src/screens/joiningscreen.cpp | 5 +++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/netplay/netsocket.cpp b/lib/netplay/netsocket.cpp index 47a45a053a0..b449acb9783 100644 --- a/lib/netplay/netsocket.cpp +++ b/lib/netplay/netsocket.cpp @@ -72,7 +72,7 @@ struct Socket SOCKET fd[SOCK_COUNT]; bool ready; - std::error_code writeErrorCode = make_network_error_code(0); + nonstd::optional writeErrorCode; bool deleteLater; char textAddress[40] = {}; @@ -650,9 +650,9 @@ net::result writeAll(Socket& sock, const void *buf, size_t size, size_t return tl::make_unexpected(make_network_error_code(EBADF)); } - if (sock.writeErrorCode) + if (sock.writeErrorCode.has_value()) { - return tl::make_unexpected(sock.writeErrorCode); + return tl::make_unexpected(sock.writeErrorCode.value()); } if (size > 0) @@ -731,7 +731,7 @@ void socketFlush(Socket& sock, uint8_t player, size_t *rawByteCount) return; // Not compressed, so don't mess with zlib. } - ASSERT(!sock.writeErrorCode, "Socket write error?? (Player: %" PRIu8 "", player); + ASSERT(!sock.writeErrorCode.has_value(), "Socket write error?? (Player: %" PRIu8 "", player); // Flush data out of zlib compression state. do diff --git a/lib/netplay/netsocket.h b/lib/netplay/netsocket.h index 32fff7019d8..a660a744645 100644 --- a/lib/netplay/netsocket.h +++ b/lib/netplay/netsocket.h @@ -26,6 +26,7 @@ #include #include +#include #include namespace net @@ -137,7 +138,6 @@ int checkSockets(const SocketSet& set, unsigned int timeout); ///< Checks which // Higher-level functions for opening a connection / socket struct OpenConnectionResult { -public: OpenConnectionResult(std::error_code ec, std::string errorString) : errorCode(ec) , errorString(errorString) @@ -146,19 +146,19 @@ struct OpenConnectionResult OpenConnectionResult(Socket* open_socket) : open_socket(open_socket) { } -public: - bool hasError() const { return static_cast(errorCode); } -public: + + bool hasError() const { return errorCode.has_value(); } + OpenConnectionResult( const OpenConnectionResult& other ) = delete; // non construction-copyable OpenConnectionResult& operator=( const OpenConnectionResult& ) = delete; // non copyable OpenConnectionResult(OpenConnectionResult&&) = default; OpenConnectionResult& operator=(OpenConnectionResult&&) = default; -public: + struct SocketDeleter { void operator()(Socket* b) { if (b) { socketClose(b); } } }; std::unique_ptr open_socket; - std::error_code errorCode; + nonstd::optional errorCode; std::string errorString; }; typedef std::function OpenConnectionToHostResultCallback; diff --git a/src/screens/joiningscreen.cpp b/src/screens/joiningscreen.cpp index c3ad785edb4..1f18c884aff 100644 --- a/src/screens/joiningscreen.cpp +++ b/src/screens/joiningscreen.cpp @@ -1149,8 +1149,9 @@ void WzJoiningGameScreen_HandlerRoot::processOpenConnectionResult(size_t connect { debug(LOG_ERROR, "%s", result.errorString.c_str()); // Done trying connections - all failed - const auto sockErrorMsg = result.errorCode.message(); - auto localizedError = astringf(_("Failed to open connection: [%d] %s"), result.errorCode.value(), sockErrorMsg.c_str()); + const auto errCode = result.errorCode.value(); + const auto sockErrorMsg = errCode.message(); + auto localizedError = astringf(_("Failed to open connection: [%d] %s"), errCode.value(), sockErrorMsg.c_str()); handleFailure(FailureDetails::makeFromInternalError(WzString::fromUtf8(localizedError))); } return; From 84c26e67e3a01c8f4fa2054ba557fbab71ed4338 Mon Sep 17 00:00:00 2001 From: past-due <30942300+past-due@users.noreply.github.com> Date: Tue, 12 Nov 2024 16:17:49 -0500 Subject: [PATCH 2/2] netplay: Tweak optional.hpp usage --- lib/netplay/netsocket.cpp | 2 +- lib/netplay/netsocket.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/netplay/netsocket.cpp b/lib/netplay/netsocket.cpp index b449acb9783..f03be08e7e1 100644 --- a/lib/netplay/netsocket.cpp +++ b/lib/netplay/netsocket.cpp @@ -72,7 +72,7 @@ struct Socket SOCKET fd[SOCK_COUNT]; bool ready; - nonstd::optional writeErrorCode; + optional writeErrorCode = nullopt; bool deleteLater; char textAddress[40] = {}; diff --git a/lib/netplay/netsocket.h b/lib/netplay/netsocket.h index a660a744645..01413011cd3 100644 --- a/lib/netplay/netsocket.h +++ b/lib/netplay/netsocket.h @@ -27,6 +27,8 @@ #include #include +using nonstd::optional; +using nonstd::nullopt; #include namespace net @@ -158,7 +160,7 @@ struct OpenConnectionResult void operator()(Socket* b) { if (b) { socketClose(b); } } }; std::unique_ptr open_socket; - nonstd::optional errorCode; + optional errorCode = nullopt; std::string errorString; }; typedef std::function OpenConnectionToHostResultCallback;