From dca36ebb871d55ca825bb0090949acb59f859f07 Mon Sep 17 00:00:00 2001 From: german77 Date: Sun, 17 Sep 2023 12:11:31 -0600 Subject: [PATCH] service: mii: Address review comments --- src/core/hle/service/mii/mii.cpp | 19 +++++++++---------- src/core/hle/service/mii/mii_database.cpp | 12 ++++++------ src/core/hle/service/mii/mii_database.h | 4 ++-- .../hle/service/mii/mii_database_manager.cpp | 4 ++-- src/core/hle/service/mii/mii_types.h | 4 ++-- src/core/hle/service/mii/types/core_data.h | 1 + src/core/hle/service/mii/types/store_data.h | 2 ++ 7 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/core/hle/service/mii/mii.cpp b/src/core/hle/service/mii/mii.cpp index dae95ea68d..8de806cfba 100644 --- a/src/core/hle/service/mii/mii.cpp +++ b/src/core/hle/service/mii/mii.cpp @@ -288,7 +288,7 @@ private: LOG_INFO(Service_Mii, "called with create_id={}, is_special={}", create_id.FormattedString(), is_special); - s32 index = manager.FindIndex(create_id, is_special); + const s32 index = manager.FindIndex(create_id, is_special); IPC::ResponseBuilder rb{ctx, 3}; rb.Push(ResultSuccess); @@ -304,7 +304,6 @@ private: new_index); Result result = ResultSuccess; - if (!is_system) { result = ResultPermissionDenied; } @@ -366,7 +365,7 @@ private: void DestroyFile(HLERequestContext& ctx) { // This calls nn::settings::fwdbg::GetSettingsItemValue("is_db_test_mode_enabled"); - bool is_db_test_mode_enabled = false; + const bool is_db_test_mode_enabled = false; LOG_INFO(Service_Mii, "called is_db_test_mode_enabled={}", is_db_test_mode_enabled); @@ -386,7 +385,7 @@ private: void DeleteFile(HLERequestContext& ctx) { // This calls nn::settings::fwdbg::GetSettingsItemValue("is_db_test_mode_enabled"); - bool is_db_test_mode_enabled = false; + const bool is_db_test_mode_enabled = false; LOG_INFO(Service_Mii, "called is_db_test_mode_enabled={}", is_db_test_mode_enabled); @@ -406,7 +405,7 @@ private: void Format(HLERequestContext& ctx) { // This calls nn::settings::fwdbg::GetSettingsItemValue("is_db_test_mode_enabled"); - bool is_db_test_mode_enabled = false; + const bool is_db_test_mode_enabled = false; LOG_INFO(Service_Mii, "called is_db_test_mode_enabled={}", is_db_test_mode_enabled); @@ -427,7 +426,7 @@ private: void IsBrokenDatabaseWithClearFlag(HLERequestContext& ctx) { LOG_DEBUG(Service_Mii, "called"); - bool is_broken_with_clear_flag{}; + bool is_broken_with_clear_flag = false; Result result = ResultSuccess; if (!is_system) { @@ -547,7 +546,7 @@ private: rb.Push(ResultSuccess); rb.PushIpcInterface(system, is_system); - LOG_CRITICAL(Service_Mii, "called"); + LOG_DEBUG(Service_Mii, "called"); } bool is_system{}; @@ -580,14 +579,14 @@ public: private: void Initialize(HLERequestContext& ctx) { - LOG_CRITICAL(Service_Mii, "called"); + LOG_INFO(Service_Mii, "called"); IPC::ResponseBuilder rb{ctx, 2}; rb.Push(ResultSuccess); } void GetCount(HLERequestContext& ctx) { - LOG_CRITICAL(Service_Mii, "called"); + LOG_DEBUG(Service_Mii, "called"); IPC::ResponseBuilder rb{ctx, 3}; rb.Push(ResultSuccess); @@ -606,4 +605,4 @@ void LoopProcess(Core::System& system) { ServerManager::RunServer(std::move(server_manager)); } -} // namespace Service::Mii \ No newline at end of file +} // namespace Service::Mii diff --git a/src/core/hle/service/mii/mii_database.cpp b/src/core/hle/service/mii/mii_database.cpp index 0899f0b45d..3803e58e28 100644 --- a/src/core/hle/service/mii/mii_database.cpp +++ b/src/core/hle/service/mii/mii_database.cpp @@ -18,7 +18,7 @@ bool NintendoFigurineDatabase::IsFull() const { StoreData NintendoFigurineDatabase::Get(std::size_t index) const { StoreData store_data = miis.at(index); - // This hack is to make external database dump compatible + // This hack is to make external database dumps compatible store_data.SetDeviceChecksum(); return store_data; @@ -60,13 +60,13 @@ Result NintendoFigurineDatabase::Move(u32 current_index, u32 new_index) { const StoreData store_data = miis[current_index]; if (new_index > current_index) { - // shift left + // Shift left const u32 index_diff = new_index - current_index; for (std::size_t i = 0; i < index_diff; i++) { miis[current_index + i] = miis[current_index + i + 1]; } } else { - // shift right + // Shift right const u32 index_diff = current_index - new_index; for (std::size_t i = 0; i < index_diff; i++) { miis[current_index - i] = miis[current_index - i - 1]; @@ -90,8 +90,8 @@ void NintendoFigurineDatabase::Add(const StoreData& store_data) { } void NintendoFigurineDatabase::Delete(u32 index) { - // shift left - s32 new_database_size = database_length - 1; + // Shift left + const s32 new_database_size = database_length - 1; if (static_cast(index) < new_database_size) { for (std::size_t i = index; i < static_cast(new_database_size); i++) { miis[i] = miis[i + 1]; @@ -103,7 +103,7 @@ void NintendoFigurineDatabase::Delete(u32 index) { } void NintendoFigurineDatabase::CleanDatabase() { - memset(miis.data(), 0, sizeof(miis)); + miis = {}; version = 1; magic = DatabaseMagic; database_length = 0; diff --git a/src/core/hle/service/mii/mii_database.h b/src/core/hle/service/mii/mii_database.h index 01764999ff..3bd240f93b 100644 --- a/src/core/hle/service/mii/mii_database.h +++ b/src/core/hle/service/mii/mii_database.h @@ -17,13 +17,13 @@ public: /// Returns the total mii count. u8 GetDatabaseLength() const; - /// Returns full if database is full. + /// Returns true if database is full. bool IsFull() const; /// Returns the mii of the specified index. StoreData Get(std::size_t index) const; - /// Returns the total mii count. Ignoring special mii. + /// Returns the total mii count. Ignoring special mii. u32 GetCount(const DatabaseSessionMetadata& metadata) const; /// Returns the index of a mii. If the mii isn't found returns false. diff --git a/src/core/hle/service/mii/mii_database_manager.cpp b/src/core/hle/service/mii/mii_database_manager.cpp index 63c411690c..c39898594a 100644 --- a/src/core/hle/service/mii/mii_database_manager.cpp +++ b/src/core/hle/service/mii/mii_database_manager.cpp @@ -15,7 +15,7 @@ #include "core/hle/service/mii/types/store_data.h" namespace Service::Mii { -constexpr std::string DbFileName = "MiiDatabase.dat"; +const char* DbFileName = "MiiDatabase.dat"; DatabaseManager::DatabaseManager() {} @@ -371,7 +371,7 @@ Result DatabaseManager::DestroyFile(DatabaseSessionMetadata& metadata) { Result DatabaseManager::DeleteFile() { const bool result = Common::FS::RemoveFile(system_save_dir / DbFileName); - // Return proper FS error here + // TODO: Return proper FS error here return result ? ResultSuccess : ResultUnknown; } diff --git a/src/core/hle/service/mii/mii_types.h b/src/core/hle/service/mii/mii_types.h index 9efe6c915c..f43efd83ce 100644 --- a/src/core/hle/service/mii/mii_types.h +++ b/src/core/hle/service/mii/mii_types.h @@ -13,6 +13,7 @@ namespace Service::Mii { +constexpr std::size_t MaxNameSize = 10; constexpr u8 MaxHeight = 127; constexpr u8 MaxBuild = 127; constexpr u8 MaxType = 1; @@ -604,8 +605,7 @@ enum class ValidationResult : u32 { }; struct Nickname { - static constexpr std::size_t MaxNameSize = 10; - std::array data; + std::array data{}; // Checks for null or dirty strings bool IsValid() const { diff --git a/src/core/hle/service/mii/types/core_data.h b/src/core/hle/service/mii/types/core_data.h index e6398f68f0..8897e4f3b3 100644 --- a/src/core/hle/service/mii/types/core_data.h +++ b/src/core/hle/service/mii/types/core_data.h @@ -214,5 +214,6 @@ private: Nickname name{}; }; static_assert(sizeof(CoreData) == 0x30, "CoreData has incorrect size."); +static_assert(std::is_trivially_copyable_v, "CoreData type must be trivially copyable."); }; // namespace Service::Mii diff --git a/src/core/hle/service/mii/types/store_data.h b/src/core/hle/service/mii/types/store_data.h index 38f534d268..ed5dfb9494 100644 --- a/src/core/hle/service/mii/types/store_data.h +++ b/src/core/hle/service/mii/types/store_data.h @@ -138,6 +138,8 @@ private: u16 device_crc{}; }; static_assert(sizeof(StoreData) == 0x44, "StoreData has incorrect size."); +static_assert(std::is_trivially_copyable_v, + "StoreData type must be trivially copyable."); struct StoreDataElement { StoreData store_data{};