Port yuzu-emu/yuzu#2511: "common/file_util: Minor cleanup" (#4782)
* 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. * 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. * 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. * 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. * 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. * common/file_util: Remove unnecessary return at end of void StripTailDirSlashes() While we're at it, also invert the conditional into a guard clause.
This commit is contained in:
		| @@ -29,12 +29,12 @@ Config::Config() { | |||||||
| Config::~Config() = default; | Config::~Config() = default; | ||||||
|  |  | ||||||
| bool Config::LoadINI(const std::string& default_contents, bool retry) { | 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 (sdl2_config->ParseError() < 0) { | ||||||
|         if (retry) { |         if (retry) { | ||||||
|             LOG_WARNING(Config, "Failed to load {}. Creating file from defaults...", location); |             LOG_WARNING(Config, "Failed to load {}. Creating file from defaults...", location); | ||||||
|             FileUtil::CreateFullPath(location); |             FileUtil::CreateFullPath(location); | ||||||
|             FileUtil::WriteStringToFile(true, default_contents, location); |             FileUtil::WriteStringToFile(true, location, default_contents); | ||||||
|             sdl2_config = std::make_unique<INIReader>(location); // Reopen file |             sdl2_config = std::make_unique<INIReader>(location); // Reopen file | ||||||
|  |  | ||||||
|             return LoadINI(default_contents, false); |             return LoadINI(default_contents, false); | ||||||
|   | |||||||
| @@ -77,16 +77,17 @@ namespace FileUtil { | |||||||
| // Remove any ending forward slashes from directory paths | // Remove any ending forward slashes from directory paths | ||||||
| // Modifies argument. | // Modifies argument. | ||||||
| static void StripTailDirSlashes(std::string& fname) { | static void StripTailDirSlashes(std::string& fname) { | ||||||
|     if (fname.length() > 1) { |     if (fname.length() <= 1) { | ||||||
|         std::size_t i = fname.length(); |         return; | ||||||
|         while (i > 0 && fname[i - 1] == DIR_SEP_CHR) |  | ||||||
|             --i; |  | ||||||
|         fname.resize(i); |  | ||||||
|     } |     } | ||||||
|     return; |  | ||||||
|  |     std::size_t i = fname.length(); | ||||||
|  |     while (i > 0 && fname[i - 1] == DIR_SEP_CHR) { | ||||||
|  |         --i; | ||||||
|  |     } | ||||||
|  |     fname.resize(i); | ||||||
| } | } | ||||||
|  |  | ||||||
| // Returns true if file filename exists |  | ||||||
| bool Exists(const std::string& filename) { | bool Exists(const std::string& filename) { | ||||||
|     struct stat file_info; |     struct stat file_info; | ||||||
|  |  | ||||||
| @@ -106,7 +107,6 @@ bool Exists(const std::string& filename) { | |||||||
|     return (result == 0); |     return (result == 0); | ||||||
| } | } | ||||||
|  |  | ||||||
| // Returns true if filename is a directory |  | ||||||
| bool IsDirectory(const std::string& filename) { | bool IsDirectory(const std::string& filename) { | ||||||
|     struct stat file_info; |     struct stat file_info; | ||||||
|  |  | ||||||
| @@ -131,8 +131,6 @@ bool IsDirectory(const std::string& filename) { | |||||||
|     return S_ISDIR(file_info.st_mode); |     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) { | bool Delete(const std::string& filename) { | ||||||
|     LOG_TRACE(Common_Filesystem, "file {}", filename); |     LOG_TRACE(Common_Filesystem, "file {}", filename); | ||||||
|  |  | ||||||
| @@ -164,7 +162,6 @@ bool Delete(const std::string& filename) { | |||||||
|     return true; |     return true; | ||||||
| } | } | ||||||
|  |  | ||||||
| // Returns true if successful, or path already exists. |  | ||||||
| bool CreateDir(const std::string& path) { | bool CreateDir(const std::string& path) { | ||||||
|     LOG_TRACE(Common_Filesystem, "directory {}", path); |     LOG_TRACE(Common_Filesystem, "directory {}", path); | ||||||
| #ifdef _WIN32 | #ifdef _WIN32 | ||||||
| @@ -193,7 +190,6 @@ bool CreateDir(const std::string& path) { | |||||||
| #endif | #endif | ||||||
| } | } | ||||||
|  |  | ||||||
| // Creates the full path of fullPath returns true on success |  | ||||||
| bool CreateFullPath(const std::string& fullPath) { | bool CreateFullPath(const std::string& fullPath) { | ||||||
|     int panicCounter = 100; |     int panicCounter = 100; | ||||||
|     LOG_TRACE(Common_Filesystem, "path {}", fullPath); |     LOG_TRACE(Common_Filesystem, "path {}", fullPath); | ||||||
| @@ -229,7 +225,6 @@ bool CreateFullPath(const std::string& fullPath) { | |||||||
|     } |     } | ||||||
| } | } | ||||||
|  |  | ||||||
| // Deletes a directory filename, returns true on success |  | ||||||
| bool DeleteDir(const std::string& filename) { | bool DeleteDir(const std::string& filename) { | ||||||
|     LOG_TRACE(Common_Filesystem, "directory {}", filename); |     LOG_TRACE(Common_Filesystem, "directory {}", filename); | ||||||
|  |  | ||||||
| @@ -251,7 +246,6 @@ bool DeleteDir(const std::string& filename) { | |||||||
|     return false; |     return false; | ||||||
| } | } | ||||||
|  |  | ||||||
| // renames file srcFilename to destFilename, returns true on success |  | ||||||
| bool Rename(const std::string& srcFilename, const std::string& destFilename) { | bool Rename(const std::string& srcFilename, const std::string& destFilename) { | ||||||
|     LOG_TRACE(Common_Filesystem, "{} --> {}", srcFilename, destFilename); |     LOG_TRACE(Common_Filesystem, "{} --> {}", srcFilename, destFilename); | ||||||
| #ifdef _WIN32 | #ifdef _WIN32 | ||||||
| @@ -267,7 +261,6 @@ bool Rename(const std::string& srcFilename, const std::string& destFilename) { | |||||||
|     return false; |     return false; | ||||||
| } | } | ||||||
|  |  | ||||||
| // copies file srcFilename to destFilename, returns true on success |  | ||||||
| bool Copy(const std::string& srcFilename, const std::string& destFilename) { | bool Copy(const std::string& srcFilename, const std::string& destFilename) { | ||||||
|     LOG_TRACE(Common_Filesystem, "{} --> {}", srcFilename, destFilename); |     LOG_TRACE(Common_Filesystem, "{} --> {}", srcFilename, destFilename); | ||||||
| #ifdef _WIN32 | #ifdef _WIN32 | ||||||
| @@ -323,7 +316,6 @@ bool Copy(const std::string& srcFilename, const std::string& destFilename) { | |||||||
| #endif | #endif | ||||||
| } | } | ||||||
|  |  | ||||||
| // Returns the size of filename (64bit) |  | ||||||
| u64 GetSize(const std::string& filename) { | u64 GetSize(const std::string& filename) { | ||||||
|     if (!Exists(filename)) { |     if (!Exists(filename)) { | ||||||
|         LOG_ERROR(Common_Filesystem, "failed {}: No such file", filename); |         LOG_ERROR(Common_Filesystem, "failed {}: No such file", filename); | ||||||
| @@ -350,7 +342,6 @@ u64 GetSize(const std::string& filename) { | |||||||
|     return 0; |     return 0; | ||||||
| } | } | ||||||
|  |  | ||||||
| // Overloaded GetSize, accepts file descriptor |  | ||||||
| u64 GetSize(const int fd) { | u64 GetSize(const int fd) { | ||||||
|     struct stat buf; |     struct stat buf; | ||||||
|     if (fstat(fd, &buf) != 0) { |     if (fstat(fd, &buf) != 0) { | ||||||
| @@ -360,7 +351,6 @@ u64 GetSize(const int fd) { | |||||||
|     return buf.st_size; |     return buf.st_size; | ||||||
| } | } | ||||||
|  |  | ||||||
| // Overloaded GetSize, accepts FILE* |  | ||||||
| u64 GetSize(FILE* f) { | u64 GetSize(FILE* f) { | ||||||
|     // can't use off_t here because it can be 32-bit |     // can't use off_t here because it can be 32-bit | ||||||
|     u64 pos = ftello(f); |     u64 pos = ftello(f); | ||||||
| @@ -376,7 +366,6 @@ u64 GetSize(FILE* f) { | |||||||
|     return size; |     return size; | ||||||
| } | } | ||||||
|  |  | ||||||
| // creates an empty file filename, returns true on success |  | ||||||
| bool CreateEmptyFile(const std::string& filename) { | bool CreateEmptyFile(const std::string& filename) { | ||||||
|     LOG_TRACE(Common_Filesystem, "{}", filename); |     LOG_TRACE(Common_Filesystem, "{}", filename); | ||||||
|  |  | ||||||
| @@ -501,7 +490,6 @@ bool DeleteDirRecursively(const std::string& directory, unsigned int recursion) | |||||||
|     return true; |     return true; | ||||||
| } | } | ||||||
|  |  | ||||||
| // Create directory and copy contents (does not overwrite existing files) |  | ||||||
| void CopyDir(const std::string& source_path, const std::string& dest_path) { | void CopyDir(const std::string& source_path, const std::string& dest_path) { | ||||||
| #ifndef _WIN32 | #ifndef _WIN32 | ||||||
|     if (source_path == dest_path) |     if (source_path == dest_path) | ||||||
| @@ -538,8 +526,7 @@ void CopyDir(const std::string& source_path, const std::string& dest_path) { | |||||||
| #endif | #endif | ||||||
| } | } | ||||||
|  |  | ||||||
| // Returns the current directory | std::optional<std::string> GetCurrentDir() { | ||||||
| std::string GetCurrentDir() { |  | ||||||
| // Get the current working directory (getcwd uses malloc) | // Get the current working directory (getcwd uses malloc) | ||||||
| #ifdef _WIN32 | #ifdef _WIN32 | ||||||
|     wchar_t* dir; |     wchar_t* dir; | ||||||
| @@ -550,7 +537,7 @@ std::string GetCurrentDir() { | |||||||
| #endif | #endif | ||||||
|     { |     { | ||||||
|         LOG_ERROR(Common_Filesystem, "GetCurrentDirectory failed: {}", GetLastErrorMsg()); |         LOG_ERROR(Common_Filesystem, "GetCurrentDirectory failed: {}", GetLastErrorMsg()); | ||||||
|         return nullptr; |         return {}; | ||||||
|     } |     } | ||||||
| #ifdef _WIN32 | #ifdef _WIN32 | ||||||
|     std::string strDir = Common::UTF16ToUTF8(dir); |     std::string strDir = Common::UTF16ToUTF8(dir); | ||||||
| @@ -561,7 +548,6 @@ std::string GetCurrentDir() { | |||||||
|     return strDir; |     return strDir; | ||||||
| } | } | ||||||
|  |  | ||||||
| // Sets the current directory to the given directory |  | ||||||
| bool SetCurrentDir(const std::string& directory) { | bool SetCurrentDir(const std::string& directory) { | ||||||
| #ifdef _WIN32 | #ifdef _WIN32 | ||||||
|     return _wchdir(Common::UTF8ToUTF16W(directory).c_str()) == 0; |     return _wchdir(Common::UTF8ToUTF16W(directory).c_str()) == 0; | ||||||
| @@ -723,8 +709,6 @@ void SetUserPath(const std::string& path) { | |||||||
|     g_paths.emplace(UserPath::DLLDir, user_path + DLL_DIR DIR_SEP); |     g_paths.emplace(UserPath::DLLDir, user_path + DLL_DIR DIR_SEP); | ||||||
| } | } | ||||||
|  |  | ||||||
| // Returns a string with a Citra 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& GetUserPath(UserPath path) { | ||||||
|     // Set up all paths and files on the first run |     // Set up all paths and files on the first run | ||||||
|     if (g_paths.empty()) |     if (g_paths.empty()) | ||||||
| @@ -732,11 +716,11 @@ const std::string& GetUserPath(UserPath path) { | |||||||
|     return g_paths[path]; |     return g_paths[path]; | ||||||
| } | } | ||||||
|  |  | ||||||
| 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) { | ||||||
|     return FileUtil::IOFile(filename, text_file ? "w" : "wb").WriteBytes(str.data(), str.size()); |     return IOFile(filename, text_file ? "w" : "wb").WriteString(str); | ||||||
| } | } | ||||||
|  |  | ||||||
| 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"); |     IOFile file(filename, text_file ? "r" : "rb"); | ||||||
|  |  | ||||||
|     if (!file) |     if (!file) | ||||||
| @@ -746,13 +730,6 @@ size_t ReadFileToString(bool text_file, const char* filename, std::string& str) | |||||||
|     return file.ReadArray(&str[0], str.size()); |     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<char, 9>& short_name, | void SplitFilename83(const std::string& filename, std::array<char, 9>& short_name, | ||||||
|                      std::array<char, 4>& extension) { |                      std::array<char, 4>& extension) { | ||||||
|     const std::string forbidden_characters = ".\"/\\[]:;=, "; |     const std::string forbidden_characters = ".\"/\\[]:;=, "; | ||||||
|   | |||||||
| @@ -9,6 +9,7 @@ | |||||||
| #include <fstream> | #include <fstream> | ||||||
| #include <functional> | #include <functional> | ||||||
| #include <limits> | #include <limits> | ||||||
|  | #include <optional> | ||||||
| #include <string> | #include <string> | ||||||
| #include <type_traits> | #include <type_traits> | ||||||
| #include <vector> | #include <vector> | ||||||
| @@ -115,7 +116,7 @@ u64 ScanDirectoryTree(const std::string& directory, FSTEntry& parent_entry, | |||||||
| bool DeleteDirRecursively(const std::string& directory, unsigned int recursion = 256); | bool DeleteDirRecursively(const std::string& directory, unsigned int recursion = 256); | ||||||
|  |  | ||||||
| // Returns the current directory | // Returns the current directory | ||||||
| std::string GetCurrentDir(); | std::optional<std::string> GetCurrentDir(); | ||||||
|  |  | ||||||
| // Create directory and copy contents (does not overwrite existing files) | // Create directory and copy contents (does not overwrite existing files) | ||||||
| void CopyDir(const std::string& source_path, const std::string& dest_path); | void CopyDir(const std::string& source_path, const std::string& dest_path); | ||||||
| @@ -141,8 +142,9 @@ const std::string& GetExeDirectory(); | |||||||
| std::string AppDataRoamingDirectory(); | std::string AppDataRoamingDirectory(); | ||||||
| #endif | #endif | ||||||
|  |  | ||||||
| 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); | ||||||
| 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 |  * Splits the filename into 8.3 format | ||||||
| @@ -228,8 +230,8 @@ public: | |||||||
|         return WriteArray(&object, 1); |         return WriteArray(&object, 1); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     std::size_t WriteString(const std::string& str) { |     std::size_t WriteString(std::string_view str) { | ||||||
|         return WriteArray(str.c_str(), str.length()); |         return WriteArray(str.data(), str.length()); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     bool IsOpen() const { |     bool IsOpen() const { | ||||||
| @@ -267,8 +269,8 @@ private: | |||||||
| template <typename T> | template <typename T> | ||||||
| void OpenFStream(T& fstream, const std::string& filename, std::ios_base::openmode openmode) { | void OpenFStream(T& fstream, const std::string& filename, std::ios_base::openmode openmode) { | ||||||
| #ifdef _MSC_VER | #ifdef _MSC_VER | ||||||
|     fstream.open(Common::UTF8ToUTF16W(filename).c_str(), openmode); |     fstream.open(Common::UTF8ToUTF16W(filename), openmode); | ||||||
| #else | #else | ||||||
|     fstream.open(filename.c_str(), openmode); |     fstream.open(filename, openmode); | ||||||
| #endif | #endif | ||||||
| } | } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Tobias
					Tobias