diff options
author | laanwj <126646+laanwj@users.noreply.github.com> | 2022-06-22 10:59:54 +0200 |
---|---|---|
committer | laanwj <126646+laanwj@users.noreply.github.com> | 2022-06-22 11:07:17 +0200 |
commit | a085a554913ae8f4ed83afac830ce6dc39c9cc65 (patch) | |
tree | e5fc3b744a6562125f67c1cff713b623fee901eb /src/util | |
parent | b1a824dd06aa58618947783edee2dd891b5204dc (diff) | |
parent | a724c39606273dfe4c6f9887ef8b77d0a98f1b34 (diff) | |
download | bitcoin-a085a554913ae8f4ed83afac830ce6dc39c9cc65.tar.xz |
Merge bitcoin/bitcoin#25428: Remove Sock::Release() and CloseSocket()
a724c39606273dfe4c6f9887ef8b77d0a98f1b34 net: rename Sock::Reset() to Sock::Close() and make it private (Vasil Dimov)
e8ff3f0c52e7512a580bc907dc72e5bb141b4217 net: remove CloseSocket() (Vasil Dimov)
175fb2670a2a24220afb3eea99b7b65b0aa89c76 net: remove now unused Sock::Release() (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
* `Sock::Release()` is unused, thus remove it
* `CloseSocket()` is only called from `Sock::Reset()`, so move the body of `CloseSocket()` inside `Sock::Reset()` and remove `CloseSocket()` - this helps to hide low level file descriptor sockets inside the `Sock` class.
* Rename `Sock::Reset()` to `Sock::Close()` and make it `private` - to be used only in the destructor and in the `Sock` assignment operator. This simplifies the public API by removing one method from it.
ACKs for top commit:
laanwj:
Code review ACK a724c39606273dfe4c6f9887ef8b77d0a98f1b34
Tree-SHA512: 4b12586642b3d049092fadcb1877132e285ec66a80af92563a7703c6970e278e0f2064fba45c7eaa78eb65db94b3641fd5e5264f7b4f61116d1a6f3333868639
Diffstat (limited to 'src/util')
-rw-r--r-- | src/util/sock.cpp | 45 | ||||
-rw-r--r-- | src/util/sock.h | 21 |
2 files changed, 24 insertions, 42 deletions
diff --git a/src/util/sock.cpp b/src/util/sock.cpp index 7d5069423a..1d44fbfdae 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -39,11 +39,11 @@ Sock::Sock(Sock&& other) other.m_socket = INVALID_SOCKET; } -Sock::~Sock() { Reset(); } +Sock::~Sock() { Close(); } Sock& Sock::operator=(Sock&& other) { - Reset(); + Close(); m_socket = other.m_socket; other.m_socket = INVALID_SOCKET; return *this; @@ -51,15 +51,6 @@ Sock& Sock::operator=(Sock&& other) SOCKET Sock::Get() const { return m_socket; } -SOCKET Sock::Release() -{ - const SOCKET s = m_socket; - m_socket = INVALID_SOCKET; - return s; -} - -void Sock::Reset() { CloseSocket(m_socket); } - ssize_t Sock::Send(const void* data, size_t len, int flags) const { return send(m_socket, static_cast<const char*>(data), len, flags); @@ -366,6 +357,22 @@ bool Sock::IsConnected(std::string& errmsg) const } } +void Sock::Close() +{ + if (m_socket == INVALID_SOCKET) { + return; + } +#ifdef WIN32 + int ret = closesocket(m_socket); +#else + int ret = close(m_socket); +#endif + if (ret) { + LogPrintf("Error closing socket %d: %s\n", m_socket, NetworkErrorString(WSAGetLastError())); + } + m_socket = INVALID_SOCKET; +} + #ifdef WIN32 std::string NetworkErrorString(int err) { @@ -389,19 +396,3 @@ std::string NetworkErrorString(int err) return SysErrorString(err); } #endif - -bool CloseSocket(SOCKET& hSocket) -{ - if (hSocket == INVALID_SOCKET) - return false; -#ifdef WIN32 - int ret = closesocket(hSocket); -#else - int ret = close(hSocket); -#endif - if (ret) { - LogPrintf("Socket close failed: %d. Error: %s\n", hSocket, NetworkErrorString(WSAGetLastError())); - } - hSocket = INVALID_SOCKET; - return ret != SOCKET_ERROR; -} diff --git a/src/util/sock.h b/src/util/sock.h index 3245820995..5ca5f1b91b 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -69,18 +69,6 @@ public: [[nodiscard]] virtual SOCKET Get() const; /** - * Get the value of the contained socket and drop ownership. It will not be closed by the - * destructor after this call. - * @return socket or INVALID_SOCKET if empty - */ - virtual SOCKET Release(); - - /** - * Close if non-empty. - */ - virtual void Reset(); - - /** * send(2) wrapper. Equivalent to `send(this->Get(), data, len, flags);`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ @@ -252,12 +240,15 @@ protected: * Contained socket. `INVALID_SOCKET` designates the object is empty. */ SOCKET m_socket; + +private: + /** + * Close `m_socket` if it is not `INVALID_SOCKET`. + */ + void Close(); }; /** Return readable error string for a network error code */ std::string NetworkErrorString(int err); -/** Close socket and set hSocket to INVALID_SOCKET */ -bool CloseSocket(SOCKET& hSocket); - #endif // BITCOIN_UTIL_SOCK_H |