From 1f129b6dcab7d9dbcf0dcf25cf9fc940cce4c1dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 25 Jan 2024 09:55:54 +0100 Subject: [PATCH] Replace "ReadFileToString" with a few semantically clearer wrappers. --- Common/ArmCPUDetect.cpp | 14 +++++++------- Common/CPUDetect.cpp | 2 +- Common/Data/Format/IniFile.cpp | 2 +- Common/File/FileUtil.cpp | 18 ++++++++++-------- Common/File/FileUtil.h | 16 ++++++++++++++-- Common/LoongArchCPUDetect.cpp | 4 ++-- Common/MipsCPUDetect.cpp | 12 ++++++------ Common/RiscVCPUDetect.cpp | 6 +++--- Core/Reporting.cpp | 3 ++- UI/CwCheatScreen.cpp | 4 ++-- UI/DriverManagerScreen.cpp | 2 +- UI/GameInfoCache.cpp | 2 +- UI/NativeApp.cpp | 8 ++++---- android/jni/TestRunner.cpp | 2 +- headless/Compare.cpp | 2 +- 15 files changed, 56 insertions(+), 41 deletions(-) diff --git a/Common/ArmCPUDetect.cpp b/Common/ArmCPUDetect.cpp index 32119f9889..eec14e46a4 100644 --- a/Common/ArmCPUDetect.cpp +++ b/Common/ArmCPUDetect.cpp @@ -90,7 +90,7 @@ const char syscpupresentfile[] = "/sys/devices/system/cpu/present"; std::string GetCPUString() { std::string procdata; - bool readSuccess = File::ReadFileToString(true, Path(procfile), procdata); + bool readSuccess = File::ReadSysTextFileToString(Path(procfile), &procdata); std::istringstream file(procdata); std::string cpu_string; @@ -113,7 +113,7 @@ std::string GetCPUString() { std::string GetCPUBrandString() { std::string procdata; - bool readSuccess = File::ReadFileToString(true, Path(procfile), procdata); + bool readSuccess = File::ReadSysTextFileToString(Path(procfile), &procdata); std::istringstream file(procdata); std::string brand_string; @@ -143,7 +143,7 @@ unsigned char GetCPUImplementer() unsigned char implementer = 0; std::string procdata; - if (!File::ReadFileToString(true, Path(procfile), procdata)) + if (!File::ReadSysTextFileToString(Path(procfile), &procdata)) return 0; std::istringstream file(procdata); @@ -166,7 +166,7 @@ unsigned short GetCPUPart() unsigned short part = 0; std::string procdata; - if (!File::ReadFileToString(true, Path(procfile), procdata)) + if (!File::ReadSysTextFileToString(Path(procfile), &procdata)) return 0; std::istringstream file(procdata); @@ -188,7 +188,7 @@ bool CheckCPUFeature(const std::string& feature) std::string line, marker = "Features\t: "; std::string procdata; - if (!File::ReadFileToString(true, Path(procfile), procdata)) + if (!File::ReadSysTextFileToString(Path(procfile), &procdata)) return false; std::istringstream file(procdata); while (std::getline(file, line)) @@ -214,7 +214,7 @@ int GetCoreCount() int cores = 1; std::string presentData; - bool presentSuccess = File::ReadFileToString(true, Path(syscpupresentfile), presentData); + bool presentSuccess = File::ReadSysTextFileToString(Path(syscpupresentfile), &presentData); std::istringstream presentFile(presentData); if (presentSuccess) { @@ -228,7 +228,7 @@ int GetCoreCount() } std::string procdata; - if (!File::ReadFileToString(true, Path(procfile), procdata)) + if (!File::ReadSysTextFileToString(Path(procfile), &procdata)) return 1; std::istringstream file(procdata); diff --git a/Common/CPUDetect.cpp b/Common/CPUDetect.cpp index b0c3625afd..057a892390 100644 --- a/Common/CPUDetect.cpp +++ b/Common/CPUDetect.cpp @@ -126,7 +126,7 @@ static std::vector ParseCPUList(const std::string &filename) { std::string data; std::vector results; - if (File::ReadFileToString(true, Path(filename), data)) { + if (File::ReadSysTextFileToString(Path(filename), &data)) { std::vector ranges; SplitString(data, ',', ranges); for (auto range : ranges) { diff --git a/Common/Data/Format/IniFile.cpp b/Common/Data/Format/IniFile.cpp index 274dadb802..3455e6d5b2 100644 --- a/Common/Data/Format/IniFile.cpp +++ b/Common/Data/Format/IniFile.cpp @@ -517,7 +517,7 @@ bool IniFile::Load(const Path &path) // Open file std::string data; - if (!File::ReadFileToString(true, path, data)) { + if (!File::ReadTextFileToString(path, &data)) { return false; } std::stringstream sstream(data); diff --git a/Common/File/FileUtil.cpp b/Common/File/FileUtil.cpp index a2ac17f9c9..cf29aef398 100644 --- a/Common/File/FileUtil.cpp +++ b/Common/File/FileUtil.cpp @@ -1147,7 +1147,7 @@ bool IOFile::Resize(uint64_t size) return m_good; } -bool ReadFileToString(bool text_file, const Path &filename, std::string &str) { +bool ReadFileToStringOptions(bool text_file, bool allowShort, const Path &filename, std::string *str) { FILE *f = File::OpenCFile(filename, text_file ? "r" : "rb"); if (!f) return false; @@ -1155,21 +1155,23 @@ bool ReadFileToString(bool text_file, const Path &filename, std::string &str) { size_t len = (size_t)File::GetFileSize(f); bool success; if (len == 0) { + // Just read until we can't read anymore. size_t totalSize = 1024; size_t totalRead = 0; do { totalSize *= 2; - str.resize(totalSize); - totalRead += fread(&str[totalRead], 1, totalSize - totalRead, f); + str->resize(totalSize); + totalRead += fread(&(*str)[totalRead], 1, totalSize - totalRead, f); } while (totalRead == totalSize); - str.resize(totalRead); + str->resize(totalRead); success = true; } else { - str.resize(len); - size_t totalRead = fread(&str[0], 1, len, f); - str.resize(totalRead); + str->resize(len); + size_t totalRead = fread(&(*str)[0], 1, len, f); + str->resize(totalRead); // Allow less, because some system files will report incorrect lengths. - success = totalRead <= len; + // Also, when reading text with CRLF, the read length may be shorter. + success = (allowShort || text_file) ? (totalRead <= len) : (totalRead == len); } fclose(f); return success; diff --git a/Common/File/FileUtil.h b/Common/File/FileUtil.h index 5b7b779e8e..a9086e49a6 100644 --- a/Common/File/FileUtil.h +++ b/Common/File/FileUtil.h @@ -205,8 +205,20 @@ private: bool WriteStringToFile(bool text_file, const std::string &str, const Path &filename); bool WriteDataToFile(bool text_file, const void* data, size_t size, const Path &filename); -bool ReadFileToString(bool text_file, const Path &filename, std::string &str); +bool ReadFileToStringOptions(bool text_file, bool allowShort, const Path &path, std::string *str); + +// Wrappers that clarify the intentions. +inline bool ReadBinaryFileToString(const Path &path, std::string *str) { + return ReadFileToStringOptions(false, false, path, str); +} +inline bool ReadSysTextFileToString(const Path &path, std::string *str) { + return ReadFileToStringOptions(true, true, path, str); +} +inline bool ReadTextFileToString(const Path &path, std::string *str) { + return ReadFileToStringOptions(true, false, path, str); +} + // Return value must be delete[]-d. -uint8_t *ReadLocalFile(const Path &filename, size_t *size); +uint8_t *ReadLocalFile(const Path &path, size_t *size); } // namespace diff --git a/Common/LoongArchCPUDetect.cpp b/Common/LoongArchCPUDetect.cpp index 3c003c986e..0382b98a38 100644 --- a/Common/LoongArchCPUDetect.cpp +++ b/Common/LoongArchCPUDetect.cpp @@ -54,7 +54,7 @@ private: LoongArchCPUInfoParser::LoongArchCPUInfoParser() { std::string procdata, line; - if (!File::ReadFileToString(true, Path(procfile), procdata)) + if (!File::ReadSysTextFileToString(Path(procfile), &procdata)) return; std::istringstream file(procdata); @@ -87,7 +87,7 @@ int LoongArchCPUInfoParser::ProcessorCount() { int LoongArchCPUInfoParser::TotalLogicalCount() { std::string presentData, line; - bool presentSuccess = File::ReadFileToString(true, Path(syscpupresentfile), presentData); + bool presentSuccess = File::ReadSysTextFileToString(Path(syscpupresentfile), &presentData); if (presentSuccess) { std::istringstream presentFile(presentData); diff --git a/Common/MipsCPUDetect.cpp b/Common/MipsCPUDetect.cpp index c569d15d94..7f69462d96 100644 --- a/Common/MipsCPUDetect.cpp +++ b/Common/MipsCPUDetect.cpp @@ -37,7 +37,7 @@ std::string GetCPUString() { std::string cpu_string = "Unknown"; std::string procdata; - if (!File::ReadFileToString(true, procfile, procdata)) + if (!File::ReadSysTextFileToString(procfile, &procdata)) return cpu_string; std::istringstream file(procdata); @@ -59,7 +59,7 @@ unsigned char GetCPUImplementer() unsigned char implementer = 0; std::string procdata; - if (!File::ReadFileToString(true, procfile, procdata)) + if (!File::ReadSysTextFileToString(procfile, &procdata)) return 0; std::istringstream file(procdata); @@ -82,7 +82,7 @@ unsigned short GetCPUPart() unsigned short part = 0; std::string procdata; - if (!File::ReadFileToString(true, procfile, procdata)) + if (!File::ReadSysTextFileToString(procfile, &procdata)) return 0; std::istringstream file(procdata); @@ -104,7 +104,7 @@ bool CheckCPUASE(const std::string& ase) std::string line, marker = "ASEs implemented\t: "; std::string procdata; - if (!File::ReadFileToString(true, procfile, procdata)) + if (!File::ReadSysTextFileToString(procfile, &procdata)) return false; std::istringstream file(procdata); @@ -131,7 +131,7 @@ int GetCoreCount() int cores = 1; std::string presentData; - bool presentSuccess = File::ReadFileToString(true, syscpupresentfile, presentData); + bool presentSuccess = File::ReadSysTextFileToString(syscpupresentfile, &presentData); std::istringstream presentFile(presentData); if (presentSuccess) { @@ -145,7 +145,7 @@ int GetCoreCount() } std::string procdata; - if (!File::ReadFileToString(true, procfile, procdata)) + if (!File::ReadSysTextFileToString(procfile, &procdata)) return 1; std::istringstream file(procdata); diff --git a/Common/RiscVCPUDetect.cpp b/Common/RiscVCPUDetect.cpp index 874c9c6a4c..cb006238ae 100644 --- a/Common/RiscVCPUDetect.cpp +++ b/Common/RiscVCPUDetect.cpp @@ -60,7 +60,7 @@ private: RiscVCPUInfoParser::RiscVCPUInfoParser() { std::string procdata, line; - if (!File::ReadFileToString(true, Path(procfile), procdata)) + if (!File::ReadSysTextFileToString(Path(procfile), &procdata)) return; std::istringstream file(procdata); @@ -94,7 +94,7 @@ int RiscVCPUInfoParser::ProcessorCount() { int RiscVCPUInfoParser::TotalLogicalCount() { std::string presentData, line; - bool presentSuccess = File::ReadFileToString(true, Path(syscpupresentfile), presentData); + bool presentSuccess = File::ReadSysTextFileToString(Path(syscpupresentfile), &presentData); if (presentSuccess) { std::istringstream presentFile(presentData); @@ -136,7 +136,7 @@ bool RiscVCPUInfoParser::FirmwareMatchesCompatible(const std::string &str) { firmwareLoaded_ = true; std::string data; - if (!File::ReadFileToString(true, Path(firmwarefile), data)) + if (!File::ReadSysTextFileToString(Path(firmwarefile), &data)) return false; SplitString(data, '\0', firmware_); diff --git a/Core/Reporting.cpp b/Core/Reporting.cpp index 3f4d8df28d..c00c2cbc2f 100644 --- a/Core/Reporting.cpp +++ b/Core/Reporting.cpp @@ -501,8 +501,9 @@ namespace Reporting void AddScreenshotData(MultipartFormDataEncoder &postdata, const Path &filename) { std::string data; - if (!filename.empty() && File::ReadFileToString(false, filename, data)) + if (!filename.empty() && File::ReadBinaryFileToString(filename, &data)) { postdata.Add("screenshot", data, "screenshot.jpg", "image/jpeg"); + } const std::string iconFilename = "disc0:/PSP_GAME/ICON0.PNG"; std::vector iconData; diff --git a/UI/CwCheatScreen.cpp b/UI/CwCheatScreen.cpp index 2ce78f44cc..12798f5c57 100644 --- a/UI/CwCheatScreen.cpp +++ b/UI/CwCheatScreen.cpp @@ -69,7 +69,7 @@ bool CwCheatScreen::TryLoadCheatInfo() { // We won't parse this, just using it to detect changes to the file. std::string str; - if (File::ReadFileToString(true, engine_->CheatFilename(), str)) { + if (File::ReadTextFileToString(engine_->CheatFilename(), &str)) { fileCheckHash_ = XXH3_64bits(str.c_str(), str.size()); } fileCheckCounter_ = 0; @@ -146,7 +146,7 @@ void CwCheatScreen::update() { if (fileCheckCounter_++ >= FILE_CHECK_FRAME_INTERVAL && engine_) { // Check if the file has changed. If it has, we'll reload. std::string str; - if (File::ReadFileToString(true, engine_->CheatFilename(), str)) { + if (File::ReadTextFileToString(engine_->CheatFilename(), &str)) { uint64_t newHash = XXH3_64bits(str.c_str(), str.size()); if (newHash != fileCheckHash_) { // This will update the hash. diff --git a/UI/DriverManagerScreen.cpp b/UI/DriverManagerScreen.cpp index fcaaa1a2f5..0e93ac3ebb 100644 --- a/UI/DriverManagerScreen.cpp +++ b/UI/DriverManagerScreen.cpp @@ -100,7 +100,7 @@ DriverChoice::DriverChoice(const std::string &driverName, bool current, UI::Layo Path metaPath = GetDriverPath() / driverName / "meta.json"; std::string metaJson; - if (File::ReadFileToString(true, metaPath, metaJson)) { + if (File::ReadTextFileToString(metaPath, &metaJson)) { std::string errorStr; meta.Read(metaJson, &errorStr); } diff --git a/UI/GameInfoCache.cpp b/UI/GameInfoCache.cpp index fed466f3c0..c67e166431 100644 --- a/UI/GameInfoCache.cpp +++ b/UI/GameInfoCache.cpp @@ -371,7 +371,7 @@ static bool ReadFileToString(IFileSystem *fs, const char *filename, std::string static bool ReadLocalFileToString(const Path &path, std::string *contents, std::mutex *mtx) { std::string data; - if (!File::ReadFileToString(false, path, data)) { + if (!File::ReadBinaryFileToString(path, &data)) { return false; } if (mtx) { diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 2578bd28f8..e81d0bdf38 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -309,7 +309,7 @@ static void CheckFailedGPUBackends() { if (System_GetPropertyBool(SYSPROP_SUPPORTS_PERMISSIONS)) { std::string data; - if (File::ReadFileToString(true, cache, data)) + if (File::ReadTextFileToString(cache, &data)) g_Config.sFailedGPUBackends = data; } @@ -433,7 +433,7 @@ void NativeInit(int argc, const char *argv[], const char *savegame_dir, const ch if (File::Exists(memstickDirFile)) { INFO_LOG(SYSTEM, "Reading '%s' to find memstick dir.", memstickDirFile.c_str()); std::string memstickDir; - if (File::ReadFileToString(true, memstickDirFile, memstickDir)) { + if (File::ReadTextFileToString(memstickDirFile, &memstickDir)) { Path memstickPath(memstickDir); if (!memstickPath.empty() && File::Exists(memstickPath)) { g_Config.memStickDirectory = memstickPath; @@ -460,7 +460,7 @@ void NativeInit(int argc, const char *argv[], const char *savegame_dir, const ch if (File::Exists(memstickDirFile)) { INFO_LOG(SYSTEM, "Reading '%s' to find memstick dir.", memstickDirFile.c_str()); std::string memstickDir; - if (File::ReadFileToString(true, memstickDirFile, memstickDir)) { + if (File::ReadTextFileToString(memstickDirFile, &memstickDir)) { Path memstickPath(memstickDir); if (!memstickPath.empty() && File::Exists(memstickPath)) { g_Config.memStickDirectory = memstickPath; @@ -1542,7 +1542,7 @@ bool NativeSaveSecret(const char *nameOfSecret, const std::string &data) { std::string NativeLoadSecret(const char *nameOfSecret) { Path path = GetSecretPath(nameOfSecret); std::string data; - if (!File::ReadFileToString(false, path, data)) { + if (!File::ReadBinaryFileToString(path, &data)) { data.clear(); // just to be sure. } return data; diff --git a/android/jni/TestRunner.cpp b/android/jni/TestRunner.cpp index 8b5a2ec5b6..691cf2e3cd 100644 --- a/android/jni/TestRunner.cpp +++ b/android/jni/TestRunner.cpp @@ -144,7 +144,7 @@ bool RunTests() { PSP_EndHostFrame(); std::string expect_results; - if (!File::ReadFileToString(true, expectedFile, expect_results)) { + if (!File::ReadTextFileToString(expectedFile, &expect_results)) { ERROR_LOG(SYSTEM, "Error opening expectedFile %s", expectedFile.c_str()); break; } diff --git a/headless/Compare.cpp b/headless/Compare.cpp index 4fd7ce8cdf..744be2babe 100644 --- a/headless/Compare.cpp +++ b/headless/Compare.cpp @@ -271,7 +271,7 @@ bool CompareOutput(const Path &bootFilename, const std::string &output, bool ver printf("%s", output.c_str()); printf("============== expected output:\n"); std::string fullExpected; - if (File::ReadFileToString(true, expect_filename, fullExpected)) + if (File::ReadTextFileToString(expect_filename, &fullExpected)) printf("%s", fullExpected.c_str()); printf("===============================\n"); }