From 7829272f7826511241defd34954e6040ea963f07 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 29 Apr 2021 18:01:03 +0200 Subject: [PATCH] net: remove now unnecessary Sock::Get() `Sock::Get()` was used only in `sock.{cpp,h}`. Remove it and access `Sock::m_socket` directly. Unit tests that used `Get()` to test for equality still verify that the behavior is correct by using the added `operator==()`. --- src/test/sock_tests.cpp | 6 +++--- src/util/sock.cpp | 7 +++++-- src/util/sock.h | 32 +++++++++++++++----------------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/test/sock_tests.cpp b/src/test/sock_tests.cpp index 26ee724bf80..38a4804fcf4 100644 --- a/src/test/sock_tests.cpp +++ b/src/test/sock_tests.cpp @@ -38,7 +38,7 @@ BOOST_AUTO_TEST_CASE(constructor_and_destructor) { const SOCKET s = CreateSocket(); Sock* sock = new Sock(s); - BOOST_CHECK_EQUAL(sock->Get(), s); + BOOST_CHECK(*sock == s); BOOST_CHECK(!SocketIsClosed(s)); delete sock; BOOST_CHECK(SocketIsClosed(s)); @@ -51,7 +51,7 @@ BOOST_AUTO_TEST_CASE(move_constructor) Sock* sock2 = new Sock(std::move(*sock1)); delete sock1; BOOST_CHECK(!SocketIsClosed(s)); - BOOST_CHECK_EQUAL(sock2->Get(), s); + BOOST_CHECK(*sock2 == s); delete sock2; BOOST_CHECK(SocketIsClosed(s)); } @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(move_assignment) *sock2 = std::move(*sock1); delete sock1; BOOST_CHECK(!SocketIsClosed(s)); - BOOST_CHECK_EQUAL(sock2->Get(), s); + BOOST_CHECK(*sock2 == s); delete sock2; BOOST_CHECK(SocketIsClosed(s)); } diff --git a/src/util/sock.cpp b/src/util/sock.cpp index fd64cae404b..e08edd42b74 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -44,8 +44,6 @@ Sock& Sock::operator=(Sock&& other) return *this; } -SOCKET Sock::Get() const { return m_socket; } - ssize_t Sock::Send(const void* data, size_t len, int flags) const { return send(m_socket, static_cast(data), len, flags); @@ -411,6 +409,11 @@ void Sock::Close() m_socket = INVALID_SOCKET; } +bool Sock::operator==(SOCKET s) const +{ + return m_socket == s; +}; + std::string NetworkErrorString(int err) { #if defined(WIN32) diff --git a/src/util/sock.h b/src/util/sock.h index 6bac2dfd346..a3d8dfe3b63 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -21,8 +21,7 @@ static constexpr auto MAX_WAIT_FOR_IO = 1s; /** - * RAII helper class that manages a socket. Mimics `std::unique_ptr`, but instead of a pointer it - * contains a socket and closes it automatically when it goes out of scope. + * RAII helper class that manages a socket and closes it automatically when it goes out of scope. */ class Sock { @@ -63,43 +62,37 @@ public: virtual Sock& operator=(Sock&& other); /** - * Get the value of the contained socket. - * @return socket or INVALID_SOCKET if empty - */ - [[nodiscard]] virtual SOCKET Get() const; - - /** - * send(2) wrapper. Equivalent to `send(this->Get(), data, len, flags);`. Code that uses this + * send(2) wrapper. Equivalent to `send(m_socket, data, len, flags);`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual ssize_t Send(const void* data, size_t len, int flags) const; /** - * recv(2) wrapper. Equivalent to `recv(this->Get(), buf, len, flags);`. Code that uses this + * recv(2) wrapper. Equivalent to `recv(m_socket, buf, len, flags);`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual ssize_t Recv(void* buf, size_t len, int flags) const; /** - * connect(2) wrapper. Equivalent to `connect(this->Get(), addr, addrlen)`. Code that uses this + * connect(2) wrapper. Equivalent to `connect(m_socket, addr, addrlen)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual int Connect(const sockaddr* addr, socklen_t addr_len) const; /** - * bind(2) wrapper. Equivalent to `bind(this->Get(), addr, addr_len)`. Code that uses this + * bind(2) wrapper. Equivalent to `bind(m_socket, addr, addr_len)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual int Bind(const sockaddr* addr, socklen_t addr_len) const; /** - * listen(2) wrapper. Equivalent to `listen(this->Get(), backlog)`. Code that uses this + * listen(2) wrapper. Equivalent to `listen(m_socket, backlog)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual int Listen(int backlog) const; /** - * accept(2) wrapper. Equivalent to `std::make_unique(accept(this->Get(), addr, addr_len))`. + * accept(2) wrapper. Equivalent to `std::make_unique(accept(m_socket, addr, addr_len))`. * Code that uses this wrapper can be unit tested if this method is overridden by a mock Sock * implementation. * The returned unique_ptr is empty if `accept()` failed in which case errno will be set. @@ -108,7 +101,7 @@ public: /** * getsockopt(2) wrapper. Equivalent to - * `getsockopt(this->Get(), level, opt_name, opt_val, opt_len)`. Code that uses this + * `getsockopt(m_socket, level, opt_name, opt_val, opt_len)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual int GetSockOpt(int level, @@ -118,7 +111,7 @@ public: /** * setsockopt(2) wrapper. Equivalent to - * `setsockopt(this->Get(), level, opt_name, opt_val, opt_len)`. Code that uses this + * `setsockopt(m_socket, level, opt_name, opt_val, opt_len)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual int SetSockOpt(int level, @@ -128,7 +121,7 @@ public: /** * getsockname(2) wrapper. Equivalent to - * `getsockname(this->Get(), name, name_len)`. Code that uses this + * `getsockname(m_socket, name, name_len)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ [[nodiscard]] virtual int GetSockName(sockaddr* name, socklen_t* name_len) const; @@ -266,6 +259,11 @@ public: */ [[nodiscard]] virtual bool IsConnected(std::string& errmsg) const; + /** + * Check if the internal socket is equal to `s`. Use only in tests. + */ + bool operator==(SOCKET s) const; + protected: /** * Contained socket. `INVALID_SOCKET` designates the object is empty.