From 2d5b7b5fb48d182fbab8e4ad69e9a552a3c07c6e Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sun, 19 Sep 2021 16:55:35 +0200 Subject: Make fs::extractZipFile thread-safe --- src/filesys.cpp | 102 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 57 insertions(+), 45 deletions(-) (limited to 'src/filesys.cpp') diff --git a/src/filesys.cpp b/src/filesys.cpp index a07370c0e..0972acbf9 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -28,11 +28,18 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "log.h" #include "config.h" #include "porting.h" +#ifndef SERVER +#include "irr_ptr.h" +#endif namespace fs { -#ifdef _WIN32 // WINDOWS +#ifdef _WIN32 + +/*********** + * Windows * + ***********/ #define _WIN32_WINNT 0x0501 #include @@ -201,7 +208,11 @@ std::string CreateTempFile() return path; } -#else // POSIX +#else + +/********* + * POSIX * + *********/ #include #include @@ -392,6 +403,10 @@ std::string CreateTempFile() #endif +/**************************** + * portable implementations * + ****************************/ + void GetRecursiveDirs(std::vector &dirs, const std::string &dir) { static const std::set chars_to_ignore = { '_', '.' }; @@ -753,69 +768,66 @@ bool safeWriteToFile(const std::string &path, const std::string &content) return true; } +#ifndef SERVER bool extractZipFile(io::IFileSystem *fs, const char *filename, const std::string &destination) { - if (!fs->addFileArchive(filename, false, false, io::EFAT_ZIP)) { + // Be careful here not to touch the global file hierarchy in Irrlicht + // since this function needs to be thread-safe! + + io::IArchiveLoader *zip_loader = nullptr; + for (u32 i = 0; i < fs->getArchiveLoaderCount(); i++) { + if (fs->getArchiveLoader(i)->isALoadableFileFormat(io::EFAT_ZIP)) { + zip_loader = fs->getArchiveLoader(i); + break; + } + } + if (!zip_loader) { + warningstream << "fs::extractZipFile(): Irrlicht said it doesn't support ZIPs." << std::endl; return false; } - sanity_check(fs->getFileArchiveCount() > 0); - - /**********************************************************************/ - /* WARNING this is not threadsafe!! */ - /**********************************************************************/ - io::IFileArchive* opened_zip = fs->getFileArchive(fs->getFileArchiveCount() - 1); - + irr_ptr opened_zip(zip_loader->createArchive(filename, false, false)); const io::IFileList* files_in_zip = opened_zip->getFileList(); - unsigned int number_of_files = files_in_zip->getFileCount(); - - for (unsigned int i=0; i < number_of_files; i++) { - std::string fullpath = destination; - fullpath += DIR_DELIM; + for (u32 i = 0; i < files_in_zip->getFileCount(); i++) { + std::string fullpath = destination + DIR_DELIM; fullpath += files_in_zip->getFullFileName(i).c_str(); std::string fullpath_dir = fs::RemoveLastPathComponent(fullpath); - if (!files_in_zip->isDirectory(i)) { - if (!fs::PathExists(fullpath_dir) && !fs::CreateAllDirs(fullpath_dir)) { - fs->removeFileArchive(fs->getFileArchiveCount()-1); - return false; - } - - io::IReadFile* toread = opened_zip->createAndOpenFile(i); + if (files_in_zip->isDirectory(i)) + continue; // ignore, we create dirs as necessary - FILE *targetfile = fopen(fullpath.c_str(),"wb"); + if (!fs::PathExists(fullpath_dir) && !fs::CreateAllDirs(fullpath_dir)) + return false; - if (targetfile == NULL) { - fs->removeFileArchive(fs->getFileArchiveCount()-1); - return false; - } + irr_ptr toread(opened_zip->createAndOpenFile(i)); - char read_buffer[1024]; - long total_read = 0; + std::ofstream os(fullpath.c_str(), std::ios::binary); + if (!os.good()) + return false; - while (total_read < toread->getSize()) { + char buffer[4096]; + long total_read = 0; - unsigned int bytes_read = - toread->read(read_buffer,sizeof(read_buffer)); - if ((bytes_read == 0 ) || - (fwrite(read_buffer, 1, bytes_read, targetfile) != bytes_read)) - { - fclose(targetfile); - fs->removeFileArchive(fs->getFileArchiveCount() - 1); - return false; - } - total_read += bytes_read; + while (total_read < toread->getSize()) { + long bytes_read = toread->read(buffer, sizeof(buffer)); + bool error = true; + if (bytes_read != 0) { + os.write(buffer, bytes_read); + error = os.fail(); } - - fclose(targetfile); + if (error) { + os.close(); + remove(fullpath.c_str()); + return false; + } + total_read += bytes_read; } - } - fs->removeFileArchive(fs->getFileArchiveCount() - 1); return true; } +#endif bool ReadFile(const std::string &path, std::string &out) { @@ -829,7 +841,7 @@ bool ReadFile(const std::string &path, std::string &out) is.seekg(0); is.read(&out[0], size); - return true; + return !is.fail(); } bool Rename(const std::string &from, const std::string &to) -- cgit v1.2.3 From 6de8d77e17017cd5cc7b065d42566b6b1cd076cc Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sun, 19 Sep 2021 18:16:53 +0200 Subject: Move instead of copy during content install if possible --- builtin/mainmenu/pkgmgr.lua | 8 ++------ src/filesys.cpp | 24 ++++++++++++++++++++++++ src/filesys.h | 4 ++++ src/script/lua_api/l_mainmenu.cpp | 30 ++++++++++++++---------------- 4 files changed, 44 insertions(+), 22 deletions(-) (limited to 'src/filesys.cpp') diff --git a/builtin/mainmenu/pkgmgr.lua b/builtin/mainmenu/pkgmgr.lua index d07dc019c..e83a93c91 100644 --- a/builtin/mainmenu/pkgmgr.lua +++ b/builtin/mainmenu/pkgmgr.lua @@ -546,11 +546,10 @@ function pkgmgr.install_dir(type, path, basename, targetpath) local from = basefolder and basefolder.path or path if targetpath then core.delete_dir(targetpath) - core.create_dir(targetpath) else targetpath = core.get_texturepath() .. DIR_DELIM .. basename end - if not core.copy_dir(from, targetpath) then + if not core.copy_dir(from, targetpath, false) then return nil, fgettext("Failed to install $1 to $2", basename, targetpath) end @@ -571,7 +570,6 @@ function pkgmgr.install_dir(type, path, basename, targetpath) -- Get destination name for modpack if targetpath then core.delete_dir(targetpath) - core.create_dir(targetpath) else local clean_path = nil if basename ~= nil then @@ -595,7 +593,6 @@ function pkgmgr.install_dir(type, path, basename, targetpath) if targetpath then core.delete_dir(targetpath) - core.create_dir(targetpath) else local targetfolder = basename if targetfolder == nil then @@ -621,14 +618,13 @@ function pkgmgr.install_dir(type, path, basename, targetpath) if targetpath then core.delete_dir(targetpath) - core.create_dir(targetpath) else targetpath = core.get_gamepath() .. DIR_DELIM .. basename end end -- Copy it - if not core.copy_dir(basefolder.path, targetpath) then + if not core.copy_dir(basefolder.path, targetpath, false) then return nil, fgettext("Failed to install $1 to $2", basename, targetpath) end diff --git a/src/filesys.cpp b/src/filesys.cpp index 0972acbf9..44f1c88b3 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -558,6 +558,30 @@ bool CopyDir(const std::string &source, const std::string &target) return false; } +bool MoveDir(const std::string &source, const std::string &target) +{ + infostream << "Moving \"" << source << "\" to \"" << target << "\"" << std::endl; + + // If target exists as empty folder delete, otherwise error + if (fs::PathExists(target)) { + if (rmdir(target.c_str()) != 0) { + errorstream << "MoveDir: target \"" << target + << "\" exists as file or non-empty folder" << std::endl; + return false; + } + } + + // Try renaming first which is instant + if (fs::Rename(source, target)) + return true; + + infostream << "MoveDir: rename not possible, will copy instead" << std::endl; + bool retval = fs::CopyDir(source, target); + if (retval) + retval &= fs::RecursiveDelete(source); + return retval; +} + bool PathStartsWith(const std::string &path, const std::string &prefix) { size_t pathsize = path.size(); diff --git a/src/filesys.h b/src/filesys.h index 233e56bba..3fa2524c3 100644 --- a/src/filesys.h +++ b/src/filesys.h @@ -106,6 +106,10 @@ bool CopyFileContents(const std::string &source, const std::string &target); // Omits files and subdirectories that start with a period bool CopyDir(const std::string &source, const std::string &target); +// Move directory and all subdirectories +// Behavior with files/subdirs that start with a period is undefined +bool MoveDir(const std::string &source, const std::string &target); + // Check if one path is prefix of another // For example, "/tmp" is a prefix of "/tmp" and "/tmp/file" but not "/tmp2" // Ignores case differences and '/' vs. '\\' on Windows diff --git a/src/script/lua_api/l_mainmenu.cpp b/src/script/lua_api/l_mainmenu.cpp index 2a6a9c32d..3d80bdafa 100644 --- a/src/script/lua_api/l_mainmenu.cpp +++ b/src/script/lua_api/l_mainmenu.cpp @@ -606,26 +606,24 @@ int ModApiMainMenu::l_copy_dir(lua_State *L) const char *destination = luaL_checkstring(L, 2); bool keep_source = true; + if (!lua_isnoneornil(L, 3)) + keep_source = readParam(L, 3); - if ((!lua_isnone(L,3)) && - (!lua_isnil(L,3))) { - keep_source = readParam(L,3); - } - - std::string absolute_destination = fs::RemoveRelativePathComponents(destination); - std::string absolute_source = fs::RemoveRelativePathComponents(source); - - if ((ModApiMainMenu::mayModifyPath(absolute_destination))) { - bool retval = fs::CopyDir(absolute_source,absolute_destination); - - if (retval && (!keep_source)) { + std::string abs_destination = fs::RemoveRelativePathComponents(destination); + std::string abs_source = fs::RemoveRelativePathComponents(source); - retval &= fs::RecursiveDelete(absolute_source); - } - lua_pushboolean(L,retval); + if (!ModApiMainMenu::mayModifyPath(abs_destination) || + (!keep_source && !ModApiMainMenu::mayModifyPath(abs_source))) { + lua_pushboolean(L, false); return 1; } - lua_pushboolean(L,false); + + bool retval; + if (keep_source) + retval = fs::CopyDir(abs_source, abs_destination); + else + retval = fs::MoveDir(abs_source, abs_destination); + lua_pushboolean(L, retval); return 1; } -- cgit v1.2.3 From c82ec8b210f613fcd5bb386a14f0a8f88591253a Mon Sep 17 00:00:00 2001 From: LoneWolfHT Date: Fri, 15 Oct 2021 09:16:09 -0700 Subject: Fix compiling on Windows with Visual Studio --- .gitignore | 4 ++++ README.md | 2 +- src/client/imagefilters.cpp | 1 + src/filesys.cpp | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) (limited to 'src/filesys.cpp') diff --git a/.gitignore b/.gitignore index a83a3718f..2e1e68157 100644 --- a/.gitignore +++ b/.gitignore @@ -107,6 +107,10 @@ CMakeDoxy* compile_commands.json *.apk *.zip +# Visual Studio +*.vcxproj* +*.sln +.vs/ # Optional user provided library folder lib/irrlichtmt diff --git a/README.md b/README.md index 30cc7fb20..372276b85 100644 --- a/README.md +++ b/README.md @@ -327,7 +327,7 @@ It is highly recommended to use vcpkg as package manager. After you successfully built vcpkg you can easily install the required libraries: ```powershell -vcpkg install zlib zstd curl[winssl] openal-soft libvorbis libogg sqlite3 freetype luajit gmp jsoncpp --triplet x64-windows +vcpkg install zlib zstd curl[winssl] openal-soft libvorbis libogg libjpeg-turbo sqlite3 freetype luajit gmp jsoncpp opengl-registry --triplet x64-windows ``` - **Don't forget about IrrlichtMt.** The easiest way is to clone it to `lib/irrlichtmt` as described in the Linux section. diff --git a/src/client/imagefilters.cpp b/src/client/imagefilters.cpp index 97ad094e5..b62e336f7 100644 --- a/src/client/imagefilters.cpp +++ b/src/client/imagefilters.cpp @@ -21,6 +21,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include #include +#include // Simple 2D bitmap class with just the functionality needed here class Bitmap { diff --git a/src/filesys.cpp b/src/filesys.cpp index 44f1c88b3..60090c801 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -45,6 +45,7 @@ namespace fs #include #include #include +#include std::vector GetDirListing(const std::string &pathstring) { -- cgit v1.2.3 From 0ea8df4d64959a7c7ec4e55b4895d6b16dad3000 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 29 Dec 2021 23:01:26 +0100 Subject: Socket-related cleanups Improve error handling on Windows and reduce the size of the `Address` class --- src/client/game.cpp | 1 - src/filesys.cpp | 2 + src/network/address.cpp | 119 ++++++++++++++++--------------------------- src/network/address.h | 35 +++++++------ src/network/socket.cpp | 74 +++++++++++++-------------- src/network/socket.h | 14 ----- src/server.cpp | 5 +- src/unittest/test_socket.cpp | 18 +++---- 8 files changed, 113 insertions(+), 155 deletions(-) (limited to 'src/filesys.cpp') diff --git a/src/client/game.cpp b/src/client/game.cpp index 853a52ecf..f62d26e8f 100644 --- a/src/client/game.cpp +++ b/src/client/game.cpp @@ -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; diff --git a/src/filesys.cpp b/src/filesys.cpp index 60090c801..ea00def6a 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -41,7 +41,9 @@ namespace fs * Windows * ***********/ +#ifndef _WIN32_WINNT #define _WIN32_WINNT 0x0501 +#endif #include #include #include diff --git a/src/network/address.cpp b/src/network/address.cpp index 05678aa62..90e561802 100644 --- a/src/network/address.cpp +++ b/src/network/address.cpp @@ -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(0)); + else if (m_addr_family == AF_INET6) + setAddress(static_cast(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; } diff --git a/src/network/address.h b/src/network/address.h index 4329c84a8..c2f5f2eef 100644 --- a/src/network/address.h +++ b/src/network/address.h @@ -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 @@ public: 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 }; diff --git a/src/network/socket.cpp b/src/network/socket.cpp index 94a9f4180..0bb7ea234 100644 --- a/src/network/socket.cpp +++ b/src/network/socket.cpp @@ -23,14 +23,11 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include #include -#include -#include #include #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 #include #define LAST_SOCKET_ERR() WSAGetLastError() -typedef SOCKET socket_t; +#define SOCKET_ERR_STR(e) itos(e) typedef int socklen_t; #else +#include #include #include #include @@ -53,7 +51,7 @@ typedef int socklen_t; #include #include #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 + (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 diff --git a/src/network/socket.h b/src/network/socket.h index e0e76f4c2..d34186b44 100644 --- a/src/network/socket.h +++ b/src/network/socket.h @@ -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 -#include -#include -#else -#include -#include -#endif - #include #include #include "address.h" @@ -53,8 +41,6 @@ public: 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); diff --git a/src/server.cpp b/src/server.cpp index c175cbcd2..a910185b9 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -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() diff --git a/src/unittest/test_socket.cpp b/src/unittest/test_socket.cpp index 6d5cf334d..620021b59 100644 --- a/src/unittest/test_socket.cpp +++ b/src/unittest/test_socket.cpp @@ -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); } } -- cgit v1.2.3