From 480d3aaf19307c393262fc1e326b7f7eee35b082 Mon Sep 17 00:00:00 2001 From: Vitor K Date: Wed, 25 Mar 2020 16:33:37 -0300 Subject: [PATCH] common: Port some changes from dolphin (#5127) * IOFile: Make the move constructor and move assignment operator noexcept Certain parts of the standard library try to determine whether or not a transfer operation should either be a copy or a move. The prevalent notion of move constructors/assignment operators is that they should not throw, they simply move an already existing resource somewhere else. This is typically done with 'std::move_if_noexcept'. Like the name says, if a type's move constructor is noexcept, then the functions retrieves an r-value reference (for move semantics), or an l-value (for copy semantics) if it is not noexcept. As IOFile deletes the copy constructor and copy assignment operators, using IOFile with certain parts of the standard library can fail in unexcepted ways (especially when used with various container implementations). This prevents that. * fix various instances of -1 being assigned to unsigned types * do not assign in conditional statements * File/IOFile: Check _tfopen_s properly * common/file_util.cpp: address review comments Co-authored-by: Lioncash Co-authored-by: Shawn Hoffman Co-authored-by: Sepalani --- src/common/file_util.cpp | 26 ++++++++++++++------------ src/common/file_util.h | 6 +++--- src/common/thread.cpp | 9 +++------ 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index d59e34daec..ce41890f77 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #include +#include #include #include #include @@ -541,11 +542,11 @@ void CopyDir(const std::string& source_path, const std::string& dest_path) { std::optional GetCurrentDir() { // Get the current working directory (getcwd uses malloc) #ifdef _WIN32 - wchar_t* dir; - if (!(dir = _wgetcwd(nullptr, 0))) { + wchar_t* dir = _wgetcwd(nullptr, 0); + if (!dir) { #else - char* dir; - if (!(dir = getcwd(nullptr, 0))) { + char* dir = getcwd(nullptr, 0); + if (!dir) { #endif LOG_ERROR(Common_Filesystem, "GetCurrentDirectory failed: {}", GetLastErrorMsg()); return {}; @@ -889,16 +890,16 @@ IOFile::~IOFile() { Close(); } -IOFile::IOFile(IOFile&& other) { +IOFile::IOFile(IOFile&& other) noexcept { Swap(other); } -IOFile& IOFile::operator=(IOFile&& other) { +IOFile& IOFile::operator=(IOFile&& other) noexcept { Swap(other); return *this; } -void IOFile::Swap(IOFile& other) { +void IOFile::Swap(IOFile& other) noexcept { std::swap(m_file, other.m_file); std::swap(m_good, other.m_good); } @@ -909,15 +910,16 @@ bool IOFile::Open(const std::string& filename, const char openmode[], int flags) if (flags != 0) { m_file = _wfsopen(Common::UTF8ToUTF16W(filename).c_str(), Common::UTF8ToUTF16W(openmode).c_str(), flags); + m_good = m_file != nullptr; } else { - _wfopen_s(&m_file, Common::UTF8ToUTF16W(filename).c_str(), - Common::UTF8ToUTF16W(openmode).c_str()); + m_good = _wfopen_s(&m_file, Common::UTF8ToUTF16W(filename).c_str(), + Common::UTF8ToUTF16W(openmode).c_str()) == 0; } #else - m_file = fopen(filename.c_str(), openmode); + m_file = std::fopen(filename.c_str(), openmode); + m_good = m_file != nullptr; #endif - m_good = IsOpen(); return m_good; } @@ -947,7 +949,7 @@ u64 IOFile::Tell() const { if (IsOpen()) return ftello(m_file); - return -1; + return std::numeric_limits::max(); } bool IOFile::Flush() { diff --git a/src/common/file_util.h b/src/common/file_util.h index 09c3cb6f05..2037db13e4 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -216,10 +216,10 @@ public: ~IOFile(); - IOFile(IOFile&& other); - IOFile& operator=(IOFile&& other); + IOFile(IOFile&& other) noexcept; + IOFile& operator=(IOFile&& other) noexcept; - void Swap(IOFile& other); + void Swap(IOFile& other) noexcept; bool Open(const std::string& filename, const char openmode[], int flags = 0); bool Close(); diff --git a/src/common/thread.cpp b/src/common/thread.cpp index fe7a420ccc..af180b43dc 100644 --- a/src/common/thread.cpp +++ b/src/common/thread.cpp @@ -28,11 +28,8 @@ namespace Common { #ifdef _MSC_VER // Sets the debugger-visible name of the current thread. -// Uses undocumented (actually, it is now documented) trick. -// http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vsdebug/html/vxtsksettingthreadname.asp - -// This is implemented much nicer in upcoming msvc++, see: -// http://msdn.microsoft.com/en-us/library/xcb2z8hs(VS.100).aspx +// Uses trick documented in: +// https://docs.microsoft.com/en-us/visualstudio/debugger/how-to-set-a-thread-name-in-native-code void SetCurrentThreadName(const char* name) { static const DWORD MS_VC_EXCEPTION = 0x406D1388; @@ -47,7 +44,7 @@ void SetCurrentThreadName(const char* name) { info.dwType = 0x1000; info.szName = name; - info.dwThreadID = -1; // dwThreadID; + info.dwThreadID = static_cast(-1); info.dwFlags = 0; __try {