Merge bitcoin/bitcoin#25421: net: convert standalone IsSelectableSocket() and SetSocketNonBlocking() to Sock methods

b527b54950 net: convert standalone SetSocketNonBlocking() to Sock::SetNonBlocking() (Vasil Dimov)
29f66f7682 moveonly: move SetSocketNonBlocking() from netbase to util/sock (Vasil Dimov)
b4bac55679 net: convert standalone IsSelectableSocket() to Sock::IsSelectable() (Vasil Dimov)
5db7d2ca0a moveonly: move IsSelectableSocket() from compat.h to sock.{h,cpp} (Vasil Dimov)

Pull request description:

  _This is a piece of #21878, chopped off to ease review._

  * convert standalone `IsSelectableSocket()` to `Sock::IsSelectable()`
  * convert standalone `SetSocketNonBlocking()` to `Sock::SetNonBlocking()`

  This further encapsulates syscalls inside the `Sock` class and makes the callers mockable.

ACKs for top commit:
  jonatack:
    ACK b527b54950 review/debug build/unit tests at each commit, cross-referenced the changes with `man select` and `man errno`, ran a signet node on the last commit with ip4/ip6//tor/i2p/cjdns and network connections were nominal
  dergoegge:
    Code review ACK b527b54950

Tree-SHA512: af783ce558c7a89e173f7ab323fb3517103d765c19b5d14de29f64706b4e1fea3653492e8ea73ae972699986aaddf2ae72c7cfaa7dad7614254283083b7d2632
This commit is contained in:
glozow
2022-10-12 15:48:35 -04:00
9 changed files with 80 additions and 34 deletions

View File

@@ -117,6 +117,34 @@ int Sock::GetSockName(sockaddr* name, socklen_t* name_len) const
return getsockname(m_socket, name, name_len);
}
bool Sock::SetNonBlocking() const
{
#ifdef WIN32
u_long on{1};
if (ioctlsocket(m_socket, FIONBIO, &on) == SOCKET_ERROR) {
return false;
}
#else
const int flags{fcntl(m_socket, F_GETFL, 0)};
if (flags == SOCKET_ERROR) {
return false;
}
if (fcntl(m_socket, F_SETFL, flags | O_NONBLOCK) == SOCKET_ERROR) {
return false;
}
#endif
return true;
}
bool Sock::IsSelectable() const
{
#if defined(USE_POLL) || defined(WIN32)
return true;
#else
return m_socket < FD_SETSIZE;
#endif
}
bool Sock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const
{
// We need a `shared_ptr` owning `this` for `WaitMany()`, but don't want
@@ -185,10 +213,10 @@ bool Sock::WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per
SOCKET socket_max{0};
for (const auto& [sock, events] : events_per_sock) {
const auto& s = sock->m_socket;
if (!IsSelectableSocket(s)) {
if (!sock->IsSelectable()) {
return false;
}
const auto& s = sock->m_socket;
if (events.requested & RECV) {
FD_SET(s, &recv);
}

View File

@@ -133,6 +133,18 @@ public:
*/
[[nodiscard]] virtual int GetSockName(sockaddr* name, socklen_t* name_len) const;
/**
* Set the non-blocking option on the socket.
* @return true if set successfully
*/
[[nodiscard]] virtual bool SetNonBlocking() const;
/**
* Check if the underlying socket can be used for `select(2)` (or the `Wait()` method).
* @return true if selectable
*/
[[nodiscard]] virtual bool IsSelectable() const;
using Event = uint8_t;
/**