From 36cdb4a544341e45155ff750c5f67ece71348a52 Mon Sep 17 00:00:00 2001 From: Niel Lebeck Date: Mon, 22 Apr 2024 21:51:09 -0700 Subject: [PATCH] Eliminate SettingsHandler's `SetBytes` and `Reset` methods Also make the `Decrypt` method private. As far as I can tell, the only motivation for exposing the `SetBytes` and `Reset` methods is to allow `CBoot::SetupWiiMemory` to use the same `SettingsHandler` instance to read settings data and then write it back. It seems cleaner to just use two separate instances, and require a given `SettingsHandler` instance to be used for either writing data to a buffer or reading data from a buffer, but not both. A natural next step is to split the `SettingsHandler` class into two classes, one for writing data and one for reading data. I've deferred that change for a future PR. --- Source/Core/Common/SettingsHandler.cpp | 23 ++++-------------- Source/Core/Common/SettingsHandler.h | 4 +--- Source/Core/Core/Boot/Boot_BS2Emu.cpp | 17 +++++++------ Source/Core/Core/IOS/DolphinDevice.cpp | 3 +-- .../UnitTests/Common/SettingsHandlerTest.cpp | 24 ------------------- 5 files changed, 14 insertions(+), 57 deletions(-) diff --git a/Source/Core/Common/SettingsHandler.cpp b/Source/Core/Common/SettingsHandler.cpp index e737a97620..b5c32a2099 100644 --- a/Source/Core/Common/SettingsHandler.cpp +++ b/Source/Core/Common/SettingsHandler.cpp @@ -17,14 +17,14 @@ namespace Common { -SettingsHandler::SettingsHandler() +SettingsHandler::SettingsHandler() : m_buffer{}, m_position{0}, m_key{INITIAL_SEED}, decoded{""} { - Reset(); } -SettingsHandler::SettingsHandler(const Buffer& buffer) +SettingsHandler::SettingsHandler(const Buffer& buffer) : SettingsHandler() { - SetBytes(buffer); + m_buffer = buffer; + Decrypt(); } const SettingsHandler::Buffer& SettingsHandler::GetBytes() const @@ -32,13 +32,6 @@ const SettingsHandler::Buffer& SettingsHandler::GetBytes() const return m_buffer; } -void SettingsHandler::SetBytes(const Buffer& buffer) -{ - Reset(); - m_buffer = buffer; - Decrypt(); -} - std::string SettingsHandler::GetValue(std::string_view key) const { constexpr char delim[] = "\n"; @@ -84,14 +77,6 @@ void SettingsHandler::Decrypt() std::erase(decoded, '\x0d'); } -void SettingsHandler::Reset() -{ - decoded = ""; - m_position = 0; - m_key = INITIAL_SEED; - m_buffer = {}; -} - void SettingsHandler::AddSetting(std::string_view key, std::string_view value) { WriteLine(fmt::format("{}={}\r\n", key, value)); diff --git a/Source/Core/Common/SettingsHandler.h b/Source/Core/Common/SettingsHandler.h index 26d0a87d72..0b8bc2044d 100644 --- a/Source/Core/Common/SettingsHandler.h +++ b/Source/Core/Common/SettingsHandler.h @@ -30,14 +30,12 @@ public: void AddSetting(std::string_view key, std::string_view value); const Buffer& GetBytes() const; - void SetBytes(const Buffer& buffer); std::string GetValue(std::string_view key) const; - void Decrypt(); - void Reset(); static std::string GenerateSerialNumber(); private: + void Decrypt(); void WriteLine(std::string_view str); void WriteByte(u8 b); diff --git a/Source/Core/Core/Boot/Boot_BS2Emu.cpp b/Source/Core/Core/Boot/Boot_BS2Emu.cpp index 9f1d7c19ca..891e7fc324 100644 --- a/Source/Core/Core/Boot/Boot_BS2Emu.cpp +++ b/Source/Core/Core/Boot/Boot_BS2Emu.cpp @@ -363,7 +363,6 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con auto entryPos = region_settings.find(SConfig::GetInstance().m_region); RegionSetting region_setting = entryPos->second; - Common::SettingsHandler gen; std::string serno; std::string model = "RVL-001(" + region_setting.area + ")"; CreateSystemMenuTitleDirs(); @@ -377,9 +376,9 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con IOS::HLE::FS::Mode::Read); if (file && file->Read(data.data(), data.size())) { - gen.SetBytes(data); - serno = gen.GetValue("SERNO"); - model = gen.GetValue("MODEL"); + Common::SettingsHandler settings_reader(data); + serno = settings_reader.GetValue("SERNO"); + model = settings_reader.GetValue("MODEL"); bool region_matches = false; if (Config::Get(Config::MAIN_OVERRIDE_REGION_SETTINGS)) @@ -388,15 +387,16 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con } else { - const std::string code = gen.GetValue("CODE"); + const std::string code = settings_reader.GetValue("CODE"); if (code.size() >= 2 && CodeRegion(code[1]) == SConfig::GetInstance().m_region) region_matches = true; } if (region_matches) { - region_setting = RegionSetting{gen.GetValue("AREA"), gen.GetValue("VIDEO"), - gen.GetValue("GAME"), gen.GetValue("CODE")}; + region_setting = + RegionSetting{settings_reader.GetValue("AREA"), settings_reader.GetValue("VIDEO"), + settings_reader.GetValue("GAME"), settings_reader.GetValue("CODE")}; } else { @@ -404,8 +404,6 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con if (parenthesis_pos != std::string::npos) model = model.substr(0, parenthesis_pos) + '(' + region_setting.area + ')'; } - - gen.Reset(); } } fs->Delete(IOS::SYSMENU_UID, IOS::SYSMENU_GID, settings_file_path); @@ -423,6 +421,7 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con INFO_LOG_FMT(BOOT, "Using serial number: {}", serno); } + Common::SettingsHandler gen; gen.AddSetting("AREA", region_setting.area); gen.AddSetting("MODEL", model); gen.AddSetting("DVD", "0"); diff --git a/Source/Core/Core/IOS/DolphinDevice.cpp b/Source/Core/Core/IOS/DolphinDevice.cpp index f2b35d60a2..79a9b0529c 100644 --- a/Source/Core/Core/IOS/DolphinDevice.cpp +++ b/Source/Core/Core/IOS/DolphinDevice.cpp @@ -138,8 +138,7 @@ IPCReply GetRealProductCode(Core::System& system, const IOCtlVRequest& request) if (!file.ReadBytes(data.data(), data.size())) return IPCReply(IPC_ENOENT); - Common::SettingsHandler gen; - gen.SetBytes(data); + Common::SettingsHandler gen(data); const std::string code = gen.GetValue("CODE"); const size_t length = std::min(request.io_vectors[0].size, code.length()); diff --git a/Source/UnitTests/Common/SettingsHandlerTest.cpp b/Source/UnitTests/Common/SettingsHandlerTest.cpp index e482531bfe..f480c0636e 100644 --- a/Source/UnitTests/Common/SettingsHandlerTest.cpp +++ b/Source/UnitTests/Common/SettingsHandlerTest.cpp @@ -75,35 +75,11 @@ TEST(SettingsHandlerTest, DecryptMultipleSettings) EXPECT_EQ(handler.GetValue("foo"), "bar"); } -TEST(SettingsHandlerTest, SetBytesOverwritesExistingBuffer) -{ - Common::SettingsHandler handler(BUFFER_A); - ASSERT_EQ(handler.GetValue("key"), "val"); - ASSERT_EQ(handler.GetValue("foo"), ""); - - handler.SetBytes(BUFFER_B); - EXPECT_EQ(handler.GetValue("foo"), "bar"); - EXPECT_EQ(handler.GetValue("key"), ""); -} - TEST(SettingsHandlerTest, GetValueOnSameInstance) { Common::SettingsHandler handler; handler.AddSetting("key", "val"); EXPECT_EQ(handler.GetValue("key"), ""); - - Common::SettingsHandler::Buffer buffer = handler.GetBytes(); - handler.SetBytes(buffer); - EXPECT_EQ(handler.GetValue("key"), "val"); -} - -TEST(SettingsHandlerTest, GetValueAfterReset) -{ - Common::SettingsHandler handler(BUFFER_A); - ASSERT_EQ(handler.GetValue("key"), "val"); - - handler.Reset(); - EXPECT_EQ(handler.GetValue("key"), ""); } TEST(SettingsHandlerTest, EncryptAddsLFOnNullChar)