From 8cd3d9be2646eda7177344045984f2baf9520ad3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 23 May 2019 13:32:49 -0400 Subject: [PATCH 1/6] common/file_util: Make IOFile's WriteString take a std::string_view We don't need to force the usage of a std::string here, and can instead use a std::string_view, which allows writing out other forms of strings (e.g. C-style strings) without any unnecessary heap allocations. --- src/common/file_util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/file_util.h b/src/common/file_util.h index 38cc7f059c..cd5a0c5fce 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -257,8 +257,8 @@ public: return WriteArray(&object, 1); } - std::size_t WriteString(const std::string& str) { - return WriteArray(str.c_str(), str.length()); + std::size_t WriteString(std::string_view str) { + return WriteArray(str.data(), str.length()); } bool IsOpen() const { From e3b25399868cd074e6b61dda73484f5b969fc3be Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 23 May 2019 13:37:44 -0400 Subject: [PATCH 2/6] common/file_util: Remove unnecessary c_str() calls The file stream open functions have supported std::string overloads since C++11, so we don't need to use c_str() here. Same behavior, less code. --- src/common/file_util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/file_util.h b/src/common/file_util.h index cd5a0c5fce..88760be2fa 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -286,8 +286,8 @@ private: template void OpenFStream(T& fstream, const std::string& filename, std::ios_base::openmode openmode) { #ifdef _MSC_VER - fstream.open(Common::UTF8ToUTF16W(filename).c_str(), openmode); + fstream.open(Common::UTF8ToUTF16W(filename), openmode); #else - fstream.open(filename.c_str(), openmode); + fstream.open(filename, openmode); #endif } From 2b1fcc8a14d93c16ce00e1a9888e3f4f839f18f9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 23 May 2019 13:52:40 -0400 Subject: [PATCH 3/6] common/file_util: Make ReadFileToString and WriteStringToFile consistent Makes the parameter ordering consistent, and also makes the filename parameter a std::string. A std::string would be constructed anyways with the previous code, as IOFile's only constructor with a filepath is one taking a std::string. We can also make WriteStringToFile's string parameter utilize a std::string_view for the string, making use of our previous changes to IOFile. --- src/common/file_util.cpp | 6 +++--- src/common/file_util.h | 4 ++-- src/yuzu_cmd/config.cpp | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index aecb66c32d..8a902dd685 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -762,11 +762,11 @@ std::string GetNANDRegistrationDir(bool system) { return GetUserPath(UserPath::NANDDir) + "user/Contents/registered/"; } -std::size_t WriteStringToFile(bool text_file, const std::string& str, const char* filename) { - return FileUtil::IOFile(filename, text_file ? "w" : "wb").WriteBytes(str.data(), str.size()); +std::size_t WriteStringToFile(bool text_file, const std::string& filename, std::string_view str) { + return IOFile(filename, text_file ? "w" : "wb").WriteString(str); } -std::size_t ReadFileToString(bool text_file, const char* filename, std::string& str) { +std::size_t ReadFileToString(bool text_file, const std::string& filename, std::string& str) { IOFile file(filename, text_file ? "r" : "rb"); if (!file.IsOpen()) diff --git a/src/common/file_util.h b/src/common/file_util.h index 88760be2fa..c325f5b498 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -146,9 +146,9 @@ const std::string& GetExeDirectory(); std::string AppDataRoamingDirectory(); #endif -std::size_t WriteStringToFile(bool text_file, const std::string& str, const char* filename); +std::size_t WriteStringToFile(bool text_file, const std::string& filename, std::string_view str); -std::size_t ReadFileToString(bool text_file, const char* filename, std::string& str); +std::size_t ReadFileToString(bool text_file, const std::string& filename, std::string& str); /** * Splits the filename into 8.3 format diff --git a/src/yuzu_cmd/config.cpp b/src/yuzu_cmd/config.cpp index d0ae058fdb..7309564275 100644 --- a/src/yuzu_cmd/config.cpp +++ b/src/yuzu_cmd/config.cpp @@ -26,12 +26,12 @@ Config::Config() { Config::~Config() = default; bool Config::LoadINI(const std::string& default_contents, bool retry) { - const char* location = this->sdl2_config_loc.c_str(); + const std::string& location = this->sdl2_config_loc; if (sdl2_config->ParseError() < 0) { if (retry) { LOG_WARNING(Config, "Failed to load {}. Creating file from defaults...", location); FileUtil::CreateFullPath(location); - FileUtil::WriteStringToFile(true, default_contents, location); + FileUtil::WriteStringToFile(true, location, default_contents); sdl2_config = std::make_unique(location); // Reopen file return LoadINI(default_contents, false); From 943f6da1ac507daec404d593a331cc37c9ab64df Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 23 May 2019 14:22:09 -0400 Subject: [PATCH 4/6] common/file_util: Remove duplicated documentation comments These are already present within the header, so they don't need to be repeated in the cpp file. --- src/common/file_util.cpp | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 8a902dd685..7403108078 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -87,7 +87,6 @@ static void StripTailDirSlashes(std::string& fname) { return; } -// Returns true if file filename exists bool Exists(const std::string& filename) { struct stat file_info; @@ -107,7 +106,6 @@ bool Exists(const std::string& filename) { return (result == 0); } -// Returns true if filename is a directory bool IsDirectory(const std::string& filename) { struct stat file_info; @@ -132,8 +130,6 @@ bool IsDirectory(const std::string& filename) { return S_ISDIR(file_info.st_mode); } -// Deletes a given filename, return true on success -// Doesn't supports deleting a directory bool Delete(const std::string& filename) { LOG_TRACE(Common_Filesystem, "file {}", filename); @@ -165,7 +161,6 @@ bool Delete(const std::string& filename) { return true; } -// Returns true if successful, or path already exists. bool CreateDir(const std::string& path) { LOG_TRACE(Common_Filesystem, "directory {}", path); #ifdef _WIN32 @@ -194,7 +189,6 @@ bool CreateDir(const std::string& path) { #endif } -// Creates the full path of fullPath returns true on success bool CreateFullPath(const std::string& fullPath) { int panicCounter = 100; LOG_TRACE(Common_Filesystem, "path {}", fullPath); @@ -230,7 +224,6 @@ bool CreateFullPath(const std::string& fullPath) { } } -// Deletes a directory filename, returns true on success bool DeleteDir(const std::string& filename) { LOG_TRACE(Common_Filesystem, "directory {}", filename); @@ -252,7 +245,6 @@ bool DeleteDir(const std::string& filename) { return false; } -// renames file srcFilename to destFilename, returns true on success bool Rename(const std::string& srcFilename, const std::string& destFilename) { LOG_TRACE(Common_Filesystem, "{} --> {}", srcFilename, destFilename); #ifdef _WIN32 @@ -268,7 +260,6 @@ bool Rename(const std::string& srcFilename, const std::string& destFilename) { return false; } -// copies file srcFilename to destFilename, returns true on success bool Copy(const std::string& srcFilename, const std::string& destFilename) { LOG_TRACE(Common_Filesystem, "{} --> {}", srcFilename, destFilename); #ifdef _WIN32 @@ -324,7 +315,6 @@ bool Copy(const std::string& srcFilename, const std::string& destFilename) { #endif } -// Returns the size of filename (64bit) u64 GetSize(const std::string& filename) { if (!Exists(filename)) { LOG_ERROR(Common_Filesystem, "failed {}: No such file", filename); @@ -351,7 +341,6 @@ u64 GetSize(const std::string& filename) { return 0; } -// Overloaded GetSize, accepts file descriptor u64 GetSize(const int fd) { struct stat buf; if (fstat(fd, &buf) != 0) { @@ -361,7 +350,6 @@ u64 GetSize(const int fd) { return buf.st_size; } -// Overloaded GetSize, accepts FILE* u64 GetSize(FILE* f) { // can't use off_t here because it can be 32-bit u64 pos = ftello(f); @@ -377,7 +365,6 @@ u64 GetSize(FILE* f) { return size; } -// creates an empty file filename, returns true on success bool CreateEmptyFile(const std::string& filename) { LOG_TRACE(Common_Filesystem, "{}", filename); @@ -502,7 +489,6 @@ bool DeleteDirRecursively(const std::string& directory, unsigned int recursion) return true; } -// Create directory and copy contents (does not overwrite existing files) void CopyDir(const std::string& source_path, const std::string& dest_path) { #ifndef _WIN32 if (source_path == dest_path) @@ -539,7 +525,6 @@ void CopyDir(const std::string& source_path, const std::string& dest_path) { #endif } -// Returns the current directory std::string GetCurrentDir() { // Get the current working directory (getcwd uses malloc) #ifdef _WIN32 @@ -561,7 +546,6 @@ std::string GetCurrentDir() { return strDir; } -// Sets the current directory to the given directory bool SetCurrentDir(const std::string& directory) { #ifdef _WIN32 return _wchdir(Common::UTF8ToUTF16W(directory).c_str()) == 0; @@ -673,8 +657,6 @@ std::string GetSysDirectory() { return sysDir; } -// Returns a string with a yuzu data dir or file in the user's home -// directory. To be used in "multi-user" mode (that is, installed). const std::string& GetUserPath(UserPath path, const std::string& new_path) { static std::unordered_map paths; auto& user_path = paths[UserPath::UserDir]; @@ -776,13 +758,6 @@ std::size_t ReadFileToString(bool text_file, const std::string& filename, std::s return file.ReadArray(&str[0], str.size()); } -/** - * Splits the filename into 8.3 format - * Loosely implemented following https://en.wikipedia.org/wiki/8.3_filename - * @param filename The normal filename to use - * @param short_name A 9-char array in which the short name will be written - * @param extension A 4-char array in which the extension will be written - */ void SplitFilename83(const std::string& filename, std::array& short_name, std::array& extension) { const std::string forbidden_characters = ".\"/\\[]:;=, "; From 11e9bee91d645cba69e936916394a0a03875c878 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 23 May 2019 14:24:11 -0400 Subject: [PATCH 5/6] common/file_util: Make GetCurrentDir() return a std::optional nullptr was being returned in the error case, which, at a glance may seem perfectly OK... until you realize that std::string has the invariant that it may not be constructed from a null pointer. This means that if this error case was ever hit, then the application would most likely crash from a thrown exception in std::string's constructor. Instead, we can change the function to return an optional value, indicating if a failure occurred. --- src/common/file_util.cpp | 4 ++-- src/common/file_util.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 7403108078..d8812837ea 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -525,7 +525,7 @@ void CopyDir(const std::string& source_path, const std::string& dest_path) { #endif } -std::string GetCurrentDir() { +std::optional GetCurrentDir() { // Get the current working directory (getcwd uses malloc) #ifdef _WIN32 wchar_t* dir; @@ -535,7 +535,7 @@ std::string GetCurrentDir() { if (!(dir = getcwd(nullptr, 0))) { #endif LOG_ERROR(Common_Filesystem, "GetCurrentDirectory failed: {}", GetLastErrorMsg()); - return nullptr; + return {}; } #ifdef _WIN32 std::string strDir = Common::UTF16ToUTF8(dir); diff --git a/src/common/file_util.h b/src/common/file_util.h index c325f5b498..cde7ddf2d1 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -118,7 +119,7 @@ u64 ScanDirectoryTree(const std::string& directory, FSTEntry& parent_entry, bool DeleteDirRecursively(const std::string& directory, unsigned int recursion = 256); // Returns the current directory -std::string GetCurrentDir(); +std::optional GetCurrentDir(); // Create directory and copy contents (does not overwrite existing files) void CopyDir(const std::string& source_path, const std::string& dest_path); From e7ab0e91278c80eee110edda0c737ea05a4f0704 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 23 May 2019 14:33:27 -0400 Subject: [PATCH 6/6] common/file_util: Remove unnecessary return at end of void StripTailDirSlashes() While we're at it, also invert the conditional into a guard clause. --- src/common/file_util.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index d8812837ea..2d9374783d 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -78,13 +78,15 @@ namespace FileUtil { // Remove any ending forward slashes from directory paths // Modifies argument. static void StripTailDirSlashes(std::string& fname) { - if (fname.length() > 1) { - std::size_t i = fname.length(); - while (i > 0 && fname[i - 1] == DIR_SEP_CHR) - --i; - fname.resize(i); + if (fname.length() <= 1) { + return; } - return; + + std::size_t i = fname.length(); + while (i > 0 && fname[i - 1] == DIR_SEP_CHR) { + --i; + } + fname.resize(i); } bool Exists(const std::string& filename) {