Merge bitcoin/bitcoin#25428: Remove Sock::Release() and CloseSocket()

a724c39606 net: rename Sock::Reset() to Sock::Close() and make it private (Vasil Dimov)
e8ff3f0c52 net: remove CloseSocket() (Vasil Dimov)
175fb2670a 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 a724c39606

Tree-SHA512: 4b12586642b3d049092fadcb1877132e285ec66a80af92563a7703c6970e278e0f2064fba45c7eaa78eb65db94b3641fd5e5264f7b4f61116d1a6f3333868639
This commit is contained in:
laanwj
2022-06-22 10:59:54 +02:00
7 changed files with 28 additions and 76 deletions

View File

@@ -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;
}

View File

@@ -68,18 +68,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