Merge pull request #12744 from nlebeck/settingshandler-split-2

Split `SettingsHandler` into separate reader and writer classes

6 files changed, 105 insertions, 110 deletions

diff --git a/Source/Core/Common/SettingsHandler.cpp b/Source/Core/Common/SettingsHandler.cpp
index b5c32a2099..6cc9f5a8fe 100644
--- a/Source/Core/Common/SettingsHandler.cpp
+++ b/Source/Core/Common/SettingsHandler.cpp
@@ -17,72 +17,72 @@
 
 namespace Common
 {
-SettingsHandler::SettingsHandler() : m_buffer{}, m_position{0}, m_key{INITIAL_SEED}, decoded{""}
+namespace
 {
-}
+// Key used to encrypt/decrypt setting.txt contents
+constexpr u32 INITIAL_SEED = 0x73B5DBFA;
+}  // namespace
 
-SettingsHandler::SettingsHandler(const Buffer& buffer) : SettingsHandler()
+SettingsWriter::SettingsWriter() : m_buffer{}, m_position{0}, m_key{INITIAL_SEED}
 {
-  m_buffer = buffer;
-  Decrypt();
 }
 
-const SettingsHandler::Buffer& SettingsHandler::GetBytes() const
+const SettingsBuffer& SettingsWriter::GetBytes() const
 {
   return m_buffer;
 }
 
-std::string SettingsHandler::GetValue(std::string_view key) const
+std::string SettingsReader::GetValue(std::string_view key) const
 {
   constexpr char delim[] = "\n";
   std::string toFind = std::string(delim).append(key).append("=");
-  size_t found = decoded.find(toFind);
+  size_t found = m_decoded.find(toFind);
 
   if (found != std::string_view::npos)
   {
-    size_t delimFound = decoded.find(delim, found + toFind.length());
+    size_t delimFound = m_decoded.find(delim, found + toFind.length());
     if (delimFound == std::string_view::npos)
-      delimFound = decoded.length() - 1;
-    return decoded.substr(found + toFind.length(), delimFound - (found + toFind.length()));
+      delimFound = m_decoded.length() - 1;
+    return m_decoded.substr(found + toFind.length(), delimFound - (found + toFind.length()));
   }
   else
   {
     toFind = std::string(key).append("=");
-    found = decoded.find(toFind);
+    found = m_decoded.find(toFind);
     if (found == 0)
     {
-      size_t delimFound = decoded.find(delim, found + toFind.length());
+      size_t delimFound = m_decoded.find(delim, found + toFind.length());
       if (delimFound == std::string_view::npos)
-        delimFound = decoded.length() - 1;
-      return decoded.substr(found + toFind.length(), delimFound - (found + toFind.length()));
+        delimFound = m_decoded.length() - 1;
+      return m_decoded.substr(found + toFind.length(), delimFound - (found + toFind.length()));
     }
   }
 
   return "";
 }
 
-void SettingsHandler::Decrypt()
+SettingsReader::SettingsReader(const SettingsBuffer& buffer) : m_decoded{""}
 {
-  while (m_position < m_buffer.size())
+  u32 key = INITIAL_SEED;
+  for (u32 position = 0; position < buffer.size(); ++position)
   {
-    decoded.push_back((u8)(m_buffer[m_position] ^ m_key));
-    m_position++;
-    m_key = (m_key >> 31) | (m_key << 1);
+    m_decoded.push_back((u8)(buffer[position] ^ key));
+    key = (key >> 31) | (key << 1);
   }
 
   // The decoded data normally uses CRLF line endings, but occasionally
   // (see the comment in WriteLine), lines can be separated by CRLFLF.
   // To handle this, we remove every CR and treat LF as the line ending.
   // (We ignore empty lines.)
-  std::erase(decoded, '\x0d');
+  std::erase(m_decoded, '\x0d');
 }
 
-void SettingsHandler::AddSetting(std::string_view key, std::string_view value)
+void SettingsWriter::AddSetting(std::string_view key, std::string_view value)
 {
   WriteLine(fmt::format("{}={}\r\n", key, value));
 }
 
-void SettingsHandler::WriteLine(std::string_view str)
+void SettingsWriter::WriteLine(std::string_view str)
 {
   const u32 old_position = m_position;
   const u32 old_key = m_key;
@@ -106,7 +106,7 @@ void SettingsHandler::WriteLine(std::string_view str)
   }
 }
 
-void SettingsHandler::WriteByte(u8 b)
+void SettingsWriter::WriteByte(u8 b)
 {
   if (m_position >= m_buffer.size())
     return;
@@ -116,7 +116,7 @@ void SettingsHandler::WriteByte(u8 b)
   m_key = (m_key >> 31) | (m_key << 1);
 }
 
-std::string SettingsHandler::GenerateSerialNumber()
+std::string SettingsWriter::GenerateSerialNumber()
 {
   const std::time_t t = std::time(nullptr);
 
diff --git a/Source/Core/Common/SettingsHandler.h b/Source/Core/Common/SettingsHandler.h
index 0b8bc2044d..e16d1155ec 100644
--- a/Source/Core/Common/SettingsHandler.h
+++ b/Source/Core/Common/SettingsHandler.h
@@ -13,34 +13,35 @@
 
 namespace Common
 {
-class SettingsHandler
+using SettingsBuffer = std::array<u8, 0x100>;
+
+class SettingsWriter
 {
 public:
-  enum
-  {
-    SETTINGS_SIZE = 0x100,
-    // Key used to encrypt/decrypt setting.txt contents
-    INITIAL_SEED = 0x73B5DBFA
-  };
-
-  using Buffer = std::array<u8, SETTINGS_SIZE>;
-  SettingsHandler();
-  explicit SettingsHandler(const Buffer& buffer);
+  SettingsWriter();
 
   void AddSetting(std::string_view key, std::string_view value);
 
-  const Buffer& GetBytes() const;
-  std::string GetValue(std::string_view key) const;
+  const SettingsBuffer& GetBytes() const;
 
   static std::string GenerateSerialNumber();
 
 private:
-  void Decrypt();
   void WriteLine(std::string_view str);
   void WriteByte(u8 b);
 
-  std::array<u8, SETTINGS_SIZE> m_buffer;
+  SettingsBuffer m_buffer;
   u32 m_position, m_key;
-  std::string decoded;
+};
+
+class SettingsReader
+{
+public:
+  explicit SettingsReader(const SettingsBuffer& buffer);
+
+  std::string GetValue(std::string_view key) const;
+
+private:
+  std::string m_decoded;
 };
 }  // namespace Common
diff --git a/Source/Core/Core/Boot/Boot_BS2Emu.cpp b/Source/Core/Core/Boot/Boot_BS2Emu.cpp
index 1f331fbe54..baf2079677 100644
--- a/Source/Core/Core/Boot/Boot_BS2Emu.cpp
+++ b/Source/Core/Core/Boot/Boot_BS2Emu.cpp
@@ -371,12 +371,12 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con
 
   const auto fs = system.GetIOS()->GetFS();
   {
-    Common::SettingsHandler::Buffer data;
+    Common::SettingsBuffer data;
     const auto file = fs->OpenFile(IOS::SYSMENU_UID, IOS::SYSMENU_GID, settings_file_path,
                                    IOS::HLE::FS::Mode::Read);
     if (file && file->Read(data.data(), data.size()))
     {
-      Common::SettingsHandler settings_reader(data);
+      const Common::SettingsReader settings_reader(data);
       serno = settings_reader.GetValue("SERNO");
       model = settings_reader.GetValue("MODEL");
 
@@ -413,7 +413,7 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con
     if (Core::WantsDeterminism())
       serno = "123456789";
     else
-      serno = Common::SettingsHandler::GenerateSerialNumber();
+      serno = Common::SettingsWriter::GenerateSerialNumber();
     INFO_LOG_FMT(BOOT, "No previous serial number found, generated one instead: {}", serno);
   }
   else
@@ -421,20 +421,21 @@ 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");
-  gen.AddSetting("MPCH", "0x7FFE");
-  gen.AddSetting("CODE", region_setting.code);
-  gen.AddSetting("SERNO", serno);
-  gen.AddSetting("VIDEO", region_setting.video);
-  gen.AddSetting("GAME", region_setting.game);
+  Common::SettingsWriter settings_writer;
+  settings_writer.AddSetting("AREA", region_setting.area);
+  settings_writer.AddSetting("MODEL", model);
+  settings_writer.AddSetting("DVD", "0");
+  settings_writer.AddSetting("MPCH", "0x7FFE");
+  settings_writer.AddSetting("CODE", region_setting.code);
+  settings_writer.AddSetting("SERNO", serno);
+  settings_writer.AddSetting("VIDEO", region_setting.video);
+  settings_writer.AddSetting("GAME", region_setting.game);
 
   constexpr IOS::HLE::FS::Mode rw_mode = IOS::HLE::FS::Mode::ReadWrite;
   const auto settings_file = fs->CreateAndOpenFile(IOS::SYSMENU_UID, IOS::SYSMENU_GID,
                                                    settings_file_path, {rw_mode, rw_mode, rw_mode});
-  if (!settings_file || !settings_file->Write(gen.GetBytes().data(), gen.GetBytes().size()))
+  if (!settings_file ||
+      !settings_file->Write(settings_writer.GetBytes().data(), settings_writer.GetBytes().size()))
   {
     PanicAlertFmtT("SetupWiiMemory: Can't create setting.txt file");
     return false;
@@ -443,7 +444,7 @@ bool CBoot::SetupWiiMemory(Core::System& system, IOS::HLE::IOSC::ConsoleType con
   auto& memory = system.GetMemory();
 
   // Write the 256 byte setting.txt to memory.
-  memory.CopyToEmu(0x3800, gen.GetBytes().data(), gen.GetBytes().size());
+  memory.CopyToEmu(0x3800, settings_writer.GetBytes().data(), settings_writer.GetBytes().size());
 
   INFO_LOG_FMT(BOOT, "Setup Wii Memory...");
 
diff --git a/Source/Core/Core/IOS/DolphinDevice.cpp b/Source/Core/Core/IOS/DolphinDevice.cpp
index 79a9b0529c..944bec41b2 100644
--- a/Source/Core/Core/IOS/DolphinDevice.cpp
+++ b/Source/Core/Core/IOS/DolphinDevice.cpp
@@ -133,13 +133,13 @@ IPCReply GetRealProductCode(Core::System& system, const IOCtlVRequest& request)
   if (!file)
     return IPCReply(IPC_ENOENT);
 
-  Common::SettingsHandler::Buffer data;
+  Common::SettingsBuffer data;
 
   if (!file.ReadBytes(data.data(), data.size()))
     return IPCReply(IPC_ENOENT);
 
-  Common::SettingsHandler gen(data);
-  const std::string code = gen.GetValue("CODE");
+  const Common::SettingsReader settings_reader(data);
+  const std::string code = settings_reader.GetValue("CODE");
 
   const size_t length = std::min<size_t>(request.io_vectors[0].size, code.length());
   if (length == 0)
diff --git a/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp b/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp
index 14553199d6..ddc99fa4c5 100644
--- a/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp
+++ b/Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp
@@ -923,7 +923,7 @@ IPCReply NetKDRequestDevice::HandleRequestRegisterUserId(const IOS::HLE::IOCtlRe
     return IPCReply{IPC_SUCCESS};
   }
 
-  Common::SettingsHandler::Buffer data;
+  Common::SettingsBuffer data;
   if (!file->Read(data.data(), data.size()))
   {
     WriteReturnValue(memory, NWC24::WC24_ERR_FILE_READ, request.buffer_out);
@@ -931,8 +931,8 @@ IPCReply NetKDRequestDevice::HandleRequestRegisterUserId(const IOS::HLE::IOCtlRe
     return IPCReply{IPC_SUCCESS};
   }
 
-  const Common::SettingsHandler gen{data};
-  const std::string serno = gen.GetValue("SERNO");
+  const Common::SettingsReader settings_reader{data};
+  const std::string serno = settings_reader.GetValue("SERNO");
   const std::string form_data =
       fmt::format("mlid=w{}&hdid={}&rgncd={}", m_config.Id(), m_ios.GetIOSC().GetDeviceId(), serno);
   const Common::HttpRequest::Response response = m_http.Post(m_config.GetAccountURL(), form_data);
@@ -1076,12 +1076,12 @@ std::optional<IPCReply> NetKDRequestDevice::IOCtl(const IOCtlRequest& request)
       const auto fs = m_ios.GetFS();
       if (const auto file = fs->OpenFile(PID_KD, PID_KD, settings_file_path, FS::Mode::Read))
       {
-        Common::SettingsHandler::Buffer data;
+        Common::SettingsBuffer data;
         if (file->Read(data.data(), data.size()))
         {
-          const Common::SettingsHandler gen{data};
-          area = gen.GetValue("AREA");
-          model = gen.GetValue("MODEL");
+          const Common::SettingsReader settings_reader{data};
+          area = settings_reader.GetValue("AREA");
+          model = settings_reader.GetValue("MODEL");
         }
       }
 
diff --git a/Source/UnitTests/Common/SettingsHandlerTest.cpp b/Source/UnitTests/Common/SettingsHandlerTest.cpp
index c8690fdf51..4b50ed85ca 100644
--- a/Source/UnitTests/Common/SettingsHandlerTest.cpp
+++ b/Source/UnitTests/Common/SettingsHandlerTest.cpp
@@ -11,15 +11,15 @@ namespace
 {
 // The encrypted bytes corresponding to the following settings, in order:
 //   "key" = "val"
-Common::SettingsHandler::Buffer BUFFER_A{0x91, 0x91, 0x90, 0xEE, 0xD1, 0x2F, 0xF0, 0x34, 0x79};
+Common::SettingsBuffer BUFFER_A{0x91, 0x91, 0x90, 0xEE, 0xD1, 0x2F, 0xF0, 0x34, 0x79};
 
 // The encrypted bytes corresponding to the following settings, in order:
 //   "key1" = "val1"
 //   "key2" = "val2"
 //   "foo" = "bar"
-Common::SettingsHandler::Buffer BUFFER_B{
-    0x91, 0x91, 0x90, 0xE2, 0x9A, 0x38, 0xFD, 0x55, 0x42, 0xEA, 0xC4, 0xF6, 0x5E, 0xF,  0xDF, 0xE7,
-    0xC3, 0x0A, 0xBB, 0x9C, 0x50, 0xB1, 0x10, 0x82, 0xB4, 0x8A, 0x0D, 0xBE, 0xCD, 0x72, 0xF4};
+Common::SettingsBuffer BUFFER_B{0x91, 0x91, 0x90, 0xE2, 0x9A, 0x38, 0xFD, 0x55, 0x42, 0xEA, 0xC4,
+                                0xF6, 0x5E, 0xF,  0xDF, 0xE7, 0xC3, 0x0A, 0xBB, 0x9C, 0x50, 0xB1,
+                                0x10, 0x82, 0xB4, 0x8A, 0x0D, 0xBE, 0xCD, 0x72, 0xF4};
 
 // The encrypted bytes corresponding to the following setting:
 //   "\xFA" = "a"
@@ -27,7 +27,7 @@ Common::SettingsHandler::Buffer BUFFER_B{
 // This setting triggers the edge case fixed in PR #8704: the key's first and only byte matches the
 // first byte of the initial encryption key, resulting in a null encoded byte on the first attempt
 // to encode the line.
-Common::SettingsHandler::Buffer BUFFER_C{0xF0, 0x0E, 0xD4, 0xB2, 0xAA, 0x44};
+Common::SettingsBuffer BUFFER_C{0xF0, 0x0E, 0xD4, 0xB2, 0xAA, 0x44};
 
 // The encrypted bytes corresponding to the following setting:
 //   "\xFA\xE9" = "a"
@@ -40,76 +40,69 @@ Common::SettingsHandler::Buffer BUFFER_C{0xF0, 0x0E, 0xD4, 0xB2, 0xAA, 0x44};
 // 2. The key's second byte matches the first byte of the encryption key after two
 // rotations, resulting in a null encoded byte on the second attempt to encode the line (with a
 // single LF inserted before the line).
-Common::SettingsHandler::Buffer BUFFER_D{0xF0, 0xFE, 0x13, 0x3A, 0x9A, 0x2F, 0x91, 0x33};
+Common::SettingsBuffer BUFFER_D{0xF0, 0xFE, 0x13, 0x3A, 0x9A, 0x2F, 0x91, 0x33};
 }  // namespace
 
-TEST(SettingsHandlerTest, EncryptSingleSetting)
+TEST(SettingsWriterTest, EncryptSingleSetting)
 {
-  Common::SettingsHandler handler;
-  handler.AddSetting("key", "val");
-  Common::SettingsHandler::Buffer buffer = handler.GetBytes();
+  Common::SettingsWriter writer;
+  writer.AddSetting("key", "val");
+  Common::SettingsBuffer buffer = writer.GetBytes();
 
   EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_A));
 }
 
-TEST(SettingsHandlerTest, DecryptSingleSetting)
+TEST(SettingsReaderTest, DecryptSingleSetting)
 {
-  Common::SettingsHandler handler(BUFFER_A);
-  EXPECT_EQ(handler.GetValue("key"), "val");
+  const Common::SettingsReader reader(BUFFER_A);
+  EXPECT_EQ(reader.GetValue("key"), "val");
 }
 
-TEST(SettingsHandlerTest, EncryptMultipleSettings)
+TEST(SettingsWriterTest, EncryptMultipleSettings)
 {
-  Common::SettingsHandler handler;
-  handler.AddSetting("key1", "val1");
-  handler.AddSetting("key2", "val2");
-  handler.AddSetting("foo", "bar");
-  Common::SettingsHandler::Buffer buffer = handler.GetBytes();
+  Common::SettingsWriter writer;
+  writer.AddSetting("key1", "val1");
+  writer.AddSetting("key2", "val2");
+  writer.AddSetting("foo", "bar");
+  Common::SettingsBuffer buffer = writer.GetBytes();
 
   EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_B));
 }
 
-TEST(SettingsHandlerTest, DecryptMultipleSettings)
+TEST(SettingsReaderTest, DecryptMultipleSettings)
 {
-  Common::SettingsHandler handler(BUFFER_B);
-  EXPECT_EQ(handler.GetValue("key1"), "val1");
-  EXPECT_EQ(handler.GetValue("key2"), "val2");
-  EXPECT_EQ(handler.GetValue("foo"), "bar");
+  const Common::SettingsReader reader(BUFFER_B);
+  EXPECT_EQ(reader.GetValue("key1"), "val1");
+  EXPECT_EQ(reader.GetValue("key2"), "val2");
+  EXPECT_EQ(reader.GetValue("foo"), "bar");
 }
 
-TEST(SettingsHandlerTest, GetValueOnSameInstance)
+TEST(SettingsWriterTest, EncryptAddsLFOnNullChar)
 {
-  Common::SettingsHandler handler;
-  handler.AddSetting("key", "val");
-  EXPECT_EQ(handler.GetValue("key"), "");
-}
-
-TEST(SettingsHandlerTest, EncryptAddsLFOnNullChar)
-{
-  Common::SettingsHandler handler;
-  handler.AddSetting("\xFA", "a");
-  Common::SettingsHandler::Buffer buffer = handler.GetBytes();
+  Common::SettingsWriter writer;
+  writer.AddSetting("\xFA", "a");
+  Common::SettingsBuffer buffer = writer.GetBytes();
 
   EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_C));
 }
 
-TEST(SettingsHandlerTest, EncryptAddsLFOnNullCharTwice)
+TEST(SettingsWriterTest, EncryptAddsLFOnNullCharTwice)
 {
-  Common::SettingsHandler handler;
-  handler.AddSetting("\xFA\xE9", "a");
-  Common::SettingsHandler::Buffer buffer = handler.GetBytes();
+  Common::SettingsWriter writer;
+  writer.AddSetting("\xFA\xE9", "a");
+  Common::SettingsBuffer buffer = writer.GetBytes();
 
   EXPECT_TRUE(std::ranges::equal(buffer, BUFFER_D));
 }
 
-TEST(SettingsHandlerTest, DecryptSingleAddedLF)
+TEST(SettingsReaderTest, DecryptSingleAddedLF)
 {
-  Common::SettingsHandler handler(BUFFER_C);
-  EXPECT_EQ(handler.GetValue("\xFA"), "a");
+  const Common::SettingsReader reader(BUFFER_C);
+  EXPECT_EQ(reader.GetValue("\xFA"), "a");
 }
 
-TEST(SettingsHandlerTest, DecryptTwoAddedLFs)
+TEST(SettingsReaderTest, DecryptTwoAddedLFs)
 {
-  Common::SettingsHandler handler(BUFFER_D);
-  EXPECT_EQ(handler.GetValue("\xFA\xE9"), "a");
+  const Common::SettingsReader reader(BUFFER_D);
+  EXPECT_EQ(reader.GetValue("\xFA\xE9"), "a");
 }