]> git.lizzy.rs Git - dragonfireclient.git/commitdiff
Socket-related cleanups
authorsfan5 <sfan5@live.de>
Wed, 29 Dec 2021 22:01:26 +0000 (23:01 +0100)
committerGitHub <noreply@github.com>
Wed, 29 Dec 2021 22:01:26 +0000 (23:01 +0100)
Improve error handling on Windows and reduce the size of the `Address` class

src/client/game.cpp
src/filesys.cpp
src/network/address.cpp
src/network/address.h
src/network/socket.cpp
src/network/socket.h
src/server.cpp
src/unittest/test_socket.cpp

index 853a52ecfb811fa224dc041996677321a9e56349..f62d26e8ff83b129c32511cb5609872a3a422a45 100644 (file)
@@ -1444,7 +1444,6 @@ bool Game::connectToServer(const GameStartData &start_data,
                connect_address.Resolve(start_data.address.c_str());
 
                if (connect_address.isZero()) { // i.e. INADDR_ANY, IN6ADDR_ANY
-                       //connect_address.Resolve("localhost");
                        if (connect_address.isIPv6()) {
                                IPv6AddressBytes addr_bytes;
                                addr_bytes.bytes[15] = 1;
index 60090c8012b4b3825316b9b33293571ed790da4c..ea00def6aa733cda2792c28ea95d36d4eff13334 100644 (file)
@@ -41,7 +41,9 @@ namespace fs
  * Windows *
  ***********/
 
+#ifndef _WIN32_WINNT
 #define _WIN32_WINNT 0x0501
+#endif
 #include <windows.h>
 #include <shlwapi.h>
 #include <io.h>
index 05678aa62cc865a2fb8c46a623a5b5b857e1cc84..90e561802c58370dd8329ad35ccaefd62a32eba5 100644 (file)
@@ -87,38 +87,31 @@ Address::Address(const IPv6AddressBytes *ipv6_bytes, u16 port)
        setPort(port);
 }
 
-// Equality (address family, address and port must be equal)
-bool Address::operator==(const Address &address)
+// Equality (address family, IP and port must be equal)
+bool Address::operator==(const Address &other)
 {
-       if (address.m_addr_family != m_addr_family || address.m_port != m_port)
+       if (other.m_addr_family != m_addr_family || other.m_port != m_port)
                return false;
 
        if (m_addr_family == AF_INET) {
-               return m_address.ipv4.sin_addr.s_addr ==
-                      address.m_address.ipv4.sin_addr.s_addr;
+               return m_address.ipv4.s_addr == other.m_address.ipv4.s_addr;
        }
 
        if (m_addr_family == AF_INET6) {
-               return memcmp(m_address.ipv6.sin6_addr.s6_addr,
-                                      address.m_address.ipv6.sin6_addr.s6_addr, 16) == 0;
+               return memcmp(m_address.ipv6.s6_addr,
+                               other.m_address.ipv6.s6_addr, 16) == 0;
        }
 
        return false;
 }
 
-bool Address::operator!=(const Address &address)
-{
-       return !(*this == address);
-}
-
 void Address::Resolve(const char *name)
 {
        if (!name || name[0] == 0) {
-               if (m_addr_family == AF_INET) {
-                       setAddress((u32)0);
-               } else if (m_addr_family == AF_INET6) {
-                       setAddress((IPv6AddressBytes *)0);
-               }
+               if (m_addr_family == AF_INET)
+                       setAddress(static_cast<u32>(0));
+               else if (m_addr_family == AF_INET6)
+                       setAddress(static_cast<IPv6AddressBytes*>(nullptr));
                return;
        }
 
@@ -126,9 +119,6 @@ void Address::Resolve(const char *name)
        memset(&hints, 0, sizeof(hints));
 
        // Setup hints
-       hints.ai_socktype = 0;
-       hints.ai_protocol = 0;
-       hints.ai_flags = 0;
        if (g_settings->getBool("enable_ipv6")) {
                // AF_UNSPEC allows both IPv6 and IPv4 addresses to be returned
                hints.ai_family = AF_UNSPEC;
@@ -145,14 +135,13 @@ void Address::Resolve(const char *name)
        if (resolved->ai_family == AF_INET) {
                struct sockaddr_in *t = (struct sockaddr_in *)resolved->ai_addr;
                m_addr_family = AF_INET;
-               m_address.ipv4 = *t;
+               m_address.ipv4 = t->sin_addr;
        } else if (resolved->ai_family == AF_INET6) {
                struct sockaddr_in6 *t = (struct sockaddr_in6 *)resolved->ai_addr;
                m_addr_family = AF_INET6;
-               m_address.ipv6 = *t;
+               m_address.ipv6 = t->sin6_addr;
        } else {
-               freeaddrinfo(resolved);
-               throw ResolveError("");
+               m_addr_family = 0;
        }
        freeaddrinfo(resolved);
 }
@@ -163,47 +152,37 @@ std::string Address::serializeString() const
 // windows XP doesnt have inet_ntop, maybe use better func
 #ifdef _WIN32
        if (m_addr_family == AF_INET) {
-               u8 a, b, c, d;
-               u32 addr;
-               addr = ntohl(m_address.ipv4.sin_addr.s_addr);
-               a = (addr & 0xFF000000) >> 24;
-               b = (addr & 0x00FF0000) >> 16;
-               c = (addr & 0x0000FF00) >> 8;
-               d = (addr & 0x000000FF);
-               return itos(a) + "." + itos(b) + "." + itos(c) + "." + itos(d);
+               return inet_ntoa(m_address.ipv4);
        } else if (m_addr_family == AF_INET6) {
                std::ostringstream os;
+               os << std::hex;
                for (int i = 0; i < 16; i += 2) {
-                       u16 section = (m_address.ipv6.sin6_addr.s6_addr[i] << 8) |
-                                     (m_address.ipv6.sin6_addr.s6_addr[i + 1]);
-                       os << std::hex << section;
+                       u16 section = (m_address.ipv6.s6_addr[i] << 8) |
+                                       (m_address.ipv6.s6_addr[i + 1]);
+                       os << section;
                        if (i < 14)
                                os << ":";
                }
                return os.str();
-       } else
-               return std::string("");
+       } else {
+               return "";
+       }
 #else
        char str[INET6_ADDRSTRLEN];
-       if (inet_ntop(m_addr_family,
-                           (m_addr_family == AF_INET)
-                                           ? (void *)&(m_address.ipv4.sin_addr)
-                                           : (void *)&(m_address.ipv6.sin6_addr),
-                           str, INET6_ADDRSTRLEN) == NULL) {
-               return std::string("");
-       }
-       return std::string(str);
+       if (inet_ntop(m_addr_family, (void*) &m_address, str, sizeof(str)) == nullptr)
+               return "";
+       return str;
 #endif
 }
 
-struct sockaddr_in Address::getAddress() const
+struct in_addr Address::getAddress() const
 {
-       return m_address.ipv4; // NOTE: NO PORT INCLUDED, use getPort()
+       return m_address.ipv4;
 }
 
-struct sockaddr_in6 Address::getAddress6() const
+struct in6_addr Address::getAddress6() const
 {
-       return m_address.ipv6; // NOTE: NO PORT INCLUDED, use getPort()
+       return m_address.ipv6;
 }
 
 u16 Address::getPort() const
@@ -211,52 +190,39 @@ u16 Address::getPort() const
        return m_port;
 }
 
-int Address::getFamily() const
-{
-       return m_addr_family;
-}
-
-bool Address::isIPv6() const
-{
-       return m_addr_family == AF_INET6;
-}
-
 bool Address::isZero() const
 {
        if (m_addr_family == AF_INET) {
-               return m_address.ipv4.sin_addr.s_addr == 0;
+               return m_address.ipv4.s_addr == 0;
        }
 
        if (m_addr_family == AF_INET6) {
                static const char zero[16] = {0};
-               return memcmp(m_address.ipv6.sin6_addr.s6_addr, zero, 16) == 0;
+               return memcmp(m_address.ipv6.s6_addr, zero, 16) == 0;
        }
+
        return false;
 }
 
 void Address::setAddress(u32 address)
 {
        m_addr_family = AF_INET;
-       m_address.ipv4.sin_family = AF_INET;
-       m_address.ipv4.sin_addr.s_addr = htonl(address);
+       m_address.ipv4.s_addr = htonl(address);
 }
 
 void Address::setAddress(u8 a, u8 b, u8 c, u8 d)
 {
-       m_addr_family = AF_INET;
-       m_address.ipv4.sin_family = AF_INET;
-       u32 addr = htonl((a << 24) | (b << 16) | (c << 8) | d);
-       m_address.ipv4.sin_addr.s_addr = addr;
+       u32 addr = (a << 24) | (b << 16) | (c << 8) | d;
+       setAddress(addr);
 }
 
 void Address::setAddress(const IPv6AddressBytes *ipv6_bytes)
 {
        m_addr_family = AF_INET6;
-       m_address.ipv6.sin6_family = AF_INET6;
        if (ipv6_bytes)
-               memcpy(m_address.ipv6.sin6_addr.s6_addr, ipv6_bytes->bytes, 16);
+               memcpy(m_address.ipv6.s6_addr, ipv6_bytes->bytes, 16);
        else
-               memset(m_address.ipv6.sin6_addr.s6_addr, 0, 16);
+               memset(m_address.ipv6.s6_addr, 0, 16);
 }
 
 void Address::setPort(u16 port)
@@ -268,23 +234,26 @@ void Address::print(std::ostream *s) const
 {
        if (m_addr_family == AF_INET6)
                *s << "[" << serializeString() << "]:" << m_port;
-       else
+       else if (m_addr_family == AF_INET)
                *s << serializeString() << ":" << m_port;
+       else
+               *s << "(undefined)";
 }
 
 bool Address::isLocalhost() const
 {
        if (isIPv6()) {
-               static const unsigned char localhost_bytes[] = {
+               static const u8 localhost_bytes[] = {
                                0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1};
-               static const unsigned char mapped_ipv4_localhost[] = {
+               static const u8 mapped_ipv4_localhost[] = {
                                0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0x7f, 0, 0, 0};
 
-               auto addr = m_address.ipv6.sin6_addr.s6_addr;
+               auto addr = m_address.ipv6.s6_addr;
 
                return memcmp(addr, localhost_bytes, 16) == 0 ||
                        memcmp(addr, mapped_ipv4_localhost, 13) == 0;
        }
 
-       return (m_address.ipv4.sin_addr.s_addr & 0xFF) == 0x7f;
+       auto addr = ntohl(m_address.ipv4.s_addr);
+       return (addr >> 24) == 0x7f;
 }
index 4329c84a84e8973eed4d5d433c2237e64444069f..c2f5f2eef9c82fae9967902aa0a92958d417e6e1 100644 (file)
@@ -36,9 +36,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "irrlichttypes.h"
 #include "networkexceptions.h"
 
-class IPv6AddressBytes
+struct IPv6AddressBytes
 {
-public:
        u8 bytes[16];
        IPv6AddressBytes() { memset(bytes, 0, 16); }
 };
@@ -50,30 +49,34 @@ class Address
        Address(u32 address, u16 port);
        Address(u8 a, u8 b, u8 c, u8 d, u16 port);
        Address(const IPv6AddressBytes *ipv6_bytes, u16 port);
+
        bool operator==(const Address &address);
-       bool operator!=(const Address &address);
+       bool operator!=(const Address &address) { return !(*this == address); }
+
+       struct in_addr getAddress() const;
+       struct in6_addr getAddress6() const;
+       u16 getPort() const;
+       int getFamily() const { return m_addr_family; }
+       bool isIPv6() const { return m_addr_family == AF_INET6; }
+       bool isZero() const;
+       void print(std::ostream *s) const;
+       std::string serializeString() const;
+       bool isLocalhost() const;
+
        // Resolve() may throw ResolveError (address is unchanged in this case)
        void Resolve(const char *name);
-       struct sockaddr_in getAddress() const;
-       unsigned short getPort() const;
+
        void setAddress(u32 address);
        void setAddress(u8 a, u8 b, u8 c, u8 d);
        void setAddress(const IPv6AddressBytes *ipv6_bytes);
-       struct sockaddr_in6 getAddress6() const;
-       int getFamily() const;
-       bool isIPv6() const;
-       bool isZero() const;
-       void setPort(unsigned short port);
-       void print(std::ostream *s) const;
-       std::string serializeString() const;
-       bool isLocalhost() const;
+       void setPort(u16 port);
 
 private:
-       unsigned int m_addr_family = 0;
+       unsigned short m_addr_family = 0;
        union
        {
-               struct sockaddr_in ipv4;
-               struct sockaddr_in6 ipv6;
+               struct in_addr ipv4;
+               struct in6_addr ipv6;
        } m_address;
        u16 m_port = 0; // Port is separate from sockaddr structures
 };
index 94a9f4180389636dbdb70c908f3d960e582c8a29..0bb7ea234f8b36f183fa32fca6868da068f355cb 100644 (file)
@@ -23,14 +23,11 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <iostream>
 #include <cstdlib>
 #include <cstring>
-#include <cerrno>
-#include <sstream>
 #include <iomanip>
 #include "util/string.h"
 #include "util/numeric.h"
 #include "constants.h"
 #include "debug.h"
-#include "settings.h"
 #include "log.h"
 
 #ifdef _WIN32
@@ -42,9 +39,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <winsock2.h>
 #include <ws2tcpip.h>
 #define LAST_SOCKET_ERR() WSAGetLastError()
-typedef SOCKET socket_t;
+#define SOCKET_ERR_STR(e) itos(e)
 typedef int socklen_t;
 #else
+#include <cerrno>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
@@ -53,7 +51,7 @@ typedef int socklen_t;
 #include <unistd.h>
 #include <arpa/inet.h>
 #define LAST_SOCKET_ERR() (errno)
-typedef int socket_t;
+#define SOCKET_ERR_STR(e) strerror(e)
 #endif
 
 // Set to true to enable verbose debug output
@@ -113,7 +111,7 @@ bool UDPSocket::init(bool ipv6, bool noExceptions)
                }
 
                throw SocketException(std::string("Failed to create socket: error ") +
-                                     itos(LAST_SOCKET_ERR()));
+                                     SOCKET_ERR_STR(LAST_SOCKET_ERR()));
        }
 
        setTimeoutMs(0);
@@ -153,40 +151,40 @@ void UDPSocket::Bind(Address addr)
        }
 
        if (addr.getFamily() != m_addr_family) {
-               static const char *errmsg =
+               const char *errmsg =
                                "Socket and bind address families do not match";
                errorstream << "Bind failed: " << errmsg << std::endl;
                throw SocketException(errmsg);
        }
 
+       int ret = 0;
+
        if (m_addr_family == AF_INET6) {
                struct sockaddr_in6 address;
                memset(&address, 0, sizeof(address));
 
-               address = addr.getAddress6();
                address.sin6_family = AF_INET6;
+               address.sin6_addr = addr.getAddress6();
                address.sin6_port = htons(addr.getPort());
 
-               if (bind(m_handle, (const struct sockaddr *)&address,
-                                   sizeof(struct sockaddr_in6)) < 0) {
-                       dstream << (int)m_handle << ": Bind failed: " << strerror(errno)
-                               << std::endl;
-                       throw SocketException("Failed to bind socket");
-               }
+               ret = bind(m_handle, (const struct sockaddr *) &address,
+                               sizeof(struct sockaddr_in6));
        } else {
                struct sockaddr_in address;
                memset(&address, 0, sizeof(address));
 
-               address = addr.getAddress();
                address.sin_family = AF_INET;
+               address.sin_addr = addr.getAddress();
                address.sin_port = htons(addr.getPort());
 
-               if (bind(m_handle, (const struct sockaddr *)&address,
-                                   sizeof(struct sockaddr_in)) < 0) {
-                       dstream << (int)m_handle << ": Bind failed: " << strerror(errno)
-                               << std::endl;
-                       throw SocketException("Failed to bind socket");
-               }
+               ret = bind(m_handle, (const struct sockaddr *) &address,
+                       sizeof(struct sockaddr_in));
+       }
+
+       if (ret < 0) {
+               dstream << (int)m_handle << ": Bind failed: "
+                       << SOCKET_ERR_STR(LAST_SOCKET_ERR()) << std::endl;
+               throw SocketException("Failed to bind socket");
        }
 }
 
@@ -233,13 +231,19 @@ void UDPSocket::Send(const Address &destination, const void *data, int size)
 
        int sent;
        if (m_addr_family == AF_INET6) {
-               struct sockaddr_in6 address = destination.getAddress6();
+               struct sockaddr_in6 address = {0};
+               address.sin6_family = AF_INET6;
+               address.sin6_addr = destination.getAddress6();
                address.sin6_port = htons(destination.getPort());
+
                sent = sendto(m_handle, (const char *)data, size, 0,
                                (struct sockaddr *)&address, sizeof(struct sockaddr_in6));
        } else {
-               struct sockaddr_in address = destination.getAddress();
+               struct sockaddr_in address = {0};
+               address.sin_family = AF_INET;
+               address.sin_addr = destination.getAddress();
                address.sin_port = htons(destination.getPort());
+
                sent = sendto(m_handle, (const char *)data, size, 0,
                                (struct sockaddr *)&address, sizeof(struct sockaddr_in));
        }
@@ -267,9 +271,9 @@ int UDPSocket::Receive(Address &sender, void *data, int size)
                        return -1;
 
                u16 address_port = ntohs(address.sin6_port);
-               IPv6AddressBytes bytes;
-               memcpy(bytes.bytes, address.sin6_addr.s6_addr, 16);
-               sender = Address(&bytes, address_port);
+               const auto *bytes = reinterpret_cast<IPv6AddressBytes*>
+                       (address.sin6_addr.s6_addr);
+               sender = Address(bytes, address_port);
        } else {
                struct sockaddr_in address;
                memset(&address, 0, sizeof(address));
@@ -341,7 +345,12 @@ bool UDPSocket::WaitData(int timeout_ms)
        if (result == 0)
                return false;
 
-       if (result < 0 && (errno == EINTR || errno == EBADF)) {
+       int e = LAST_SOCKET_ERR();
+#ifdef _WIN32
+       if (result < 0 && (e == WSAEINTR || e == WSAEBADF)) {
+#else
+       if (result < 0 && (e == EINTR || e == EBADF)) {
+#endif
                // N.B. select() fails when sockets are destroyed on Connection's dtor
                // with EBADF.  Instead of doing tricky synchronization, allow this
                // thread to exit but don't throw an exception.
@@ -349,18 +358,9 @@ bool UDPSocket::WaitData(int timeout_ms)
        }
 
        if (result < 0) {
-               dstream << m_handle << ": Select failed: " << strerror(errno)
+               dstream << (int)m_handle << ": Select failed: " << SOCKET_ERR_STR(e)
                        << std::endl;
 
-#ifdef _WIN32
-               int e = WSAGetLastError();
-               dstream << (int)m_handle << ": WSAGetLastError()=" << e << std::endl;
-               if (e == 10004 /* WSAEINTR */ || e == 10009 /* WSAEBADF */) {
-                       infostream << "Ignoring WSAEINTR/WSAEBADF." << std::endl;
-                       return false;
-               }
-#endif
-
                throw SocketException("Select failed");
        } else if (!FD_ISSET(m_handle, &readset)) {
                // No data
index e0e76f4c2f176a1e8d21ecb974d688a1f0c2d402..d34186b443c348cdc8a70fc397b2b647e67b6940 100644 (file)
@@ -19,18 +19,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #pragma once
 
-#ifdef _WIN32
-#ifndef _WIN32_WINNT
-#define _WIN32_WINNT 0x0501
-#endif
-#include <windows.h>
-#include <winsock2.h>
-#include <ws2tcpip.h>
-#else
-#include <sys/socket.h>
-#include <netinet/in.h>
-#endif
-
 #include <ostream>
 #include <cstring>
 #include "address.h"
@@ -53,8 +41,6 @@ class UDPSocket
 
        bool init(bool ipv6, bool noExceptions = false);
 
-       // void Close();
-       // bool IsOpen();
        void Send(const Address &destination, const void *data, int size);
        // Returns -1 if there is no data
        int Receive(Address &sender, void *data, int size);
index c175cbcd29f3cbf9eec7e6b411c0a9e64b5943dc..a910185b982ad6b35d4e303095ce63aa94ee1982 100644 (file)
@@ -507,8 +507,9 @@ void Server::start()
                << "      \\/        \\/     \\/          \\/     \\/        " << std::endl;
        actionstream << "World at [" << m_path_world << "]" << std::endl;
        actionstream << "Server for gameid=\"" << m_gamespec.id
-                       << "\" listening on " << m_bind_addr.serializeString() << ":"
-                       << m_bind_addr.getPort() << "." << std::endl;
+                       << "\" listening on ";
+       m_bind_addr.print(&actionstream);
+       actionstream << "." << std::endl;
 }
 
 void Server::stop()
index 6d5cf334d95fb740a03056045ec48e636c65e188..620021b592aa223517cf747932c0b418715087cb 100644 (file)
@@ -97,11 +97,11 @@ void TestSocket::testIPv4Socket()
        UASSERT(strncmp(sendbuffer, rcvbuffer, sizeof(sendbuffer)) == 0);
 
        if (address != Address(0, 0, 0, 0, port)) {
-               UASSERT(sender.getAddress().sin_addr.s_addr ==
-                               address.getAddress().sin_addr.s_addr);
+               UASSERT(sender.getAddress().s_addr ==
+                               address.getAddress().s_addr);
        } else {
-               UASSERT(sender.getAddress().sin_addr.s_addr ==
-                               Address(127, 0, 0, 1, 0).getAddress().sin_addr.s_addr);
+               UASSERT(sender.getAddress().s_addr ==
+                               Address(127, 0, 0, 1, 0).getAddress().s_addr);
        }
 }
 
@@ -128,7 +128,7 @@ void TestSocket::testIPv6Socket()
 
        socket6.Bind(address6);
 
-       try {
+       {
                socket6.Send(Address(&bytes, port), sendbuffer, sizeof(sendbuffer));
 
                sleep_ms(50);
@@ -142,10 +142,8 @@ void TestSocket::testIPv6Socket()
                }
                //FIXME: This fails on some systems
                UASSERT(strncmp(sendbuffer, rcvbuffer, sizeof(sendbuffer)) == 0);
-               UASSERT(memcmp(sender.getAddress6().sin6_addr.s6_addr,
-                               Address(&bytes, 0).getAddress6().sin6_addr.s6_addr, 16) == 0);
-       } catch (SendFailedException &e) {
-               errorstream << "IPv6 support enabled but not available!"
-                                       << std::endl;
+
+               UASSERT(memcmp(sender.getAddress6().s6_addr,
+                               Address(&bytes, 0).getAddress6().s6_addr, 16) == 0);
        }
 }