From c21dad9e83e57570196fe66632f31e135f88406d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 4 Jan 2017 17:47:37 -0500 Subject: [PATCH 1/4] IOFile: Get rid of unnecessary unimplemented copy constructor/assignment operator. IOFile already inherits NonCopyable. --- Source/Core/Common/FileUtil.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index 1c38bb38d9..763da70404 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -232,10 +232,6 @@ public: std::FILE* m_file; bool m_good; - -private: - IOFile(IOFile&); - IOFile& operator=(IOFile& other); }; } // namespace From c541e1099edd1dad921d2f3937fafa602186c0a2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 4 Jan 2017 17:48:44 -0500 Subject: [PATCH 2/4] IOFile: Make class variables private Internals shouldn't be directly exposed. --- Source/Core/Common/FileUtil.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index 763da70404..c8524254c2 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -230,6 +230,7 @@ public: std::clearerr(m_file); } +private: std::FILE* m_file; bool m_good; }; From 9b8f5bce22173f66aafa5b008ff6c5b96cdb4994 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 4 Jan 2017 18:02:39 -0500 Subject: [PATCH 3/4] IOFile: Change 'operator void*' into 'explicit operator bool' 'operator void*' is basically a pre-C++11-ism that was used, as C++03 only had the notion of implicit type-conversion operators, but not explicit type conversion operators (allowing implicit conversion of a file handle to bool can go downhill pretty quickly). --- Source/Core/Common/FileUtil.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index c8524254c2..445c7fc0f4 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -211,7 +211,7 @@ public: bool IsOpen() const { return nullptr != m_file; } // m_good is set to false when a read, write or other function fails bool IsGood() const { return m_good; } - operator void*() { return m_good ? m_file : nullptr; } + explicit operator bool() const { return IsGood(); } std::FILE* ReleaseHandle(); std::FILE* GetHandle() { return m_file; } From 045a8400e6ddb854a79eaaa499d09d54345d3dc3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 4 Jan 2017 18:15:53 -0500 Subject: [PATCH 4/4] 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. --- Source/Core/Common/FileUtil.cpp | 6 +++--- Source/Core/Common/FileUtil.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/Core/Common/FileUtil.cpp b/Source/Core/Common/FileUtil.cpp index 1fae3418f5..4094f5edcf 100644 --- a/Source/Core/Common/FileUtil.cpp +++ b/Source/Core/Common/FileUtil.cpp @@ -908,18 +908,18 @@ IOFile::~IOFile() Close(); } -IOFile::IOFile(IOFile&& other) : m_file(nullptr), m_good(true) +IOFile::IOFile(IOFile&& other) noexcept : m_file(nullptr), m_good(true) { 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); diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index 445c7fc0f4..ef012ad4ff 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -168,10 +168,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[]); bool Close();