From a1f77fd14b16fb91ab4d313c3d02e76532ade578 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 27 May 2019 10:09:53 -0400 Subject: [PATCH 1/6] DiscIO/VolumeVerifier: Make use of unused variable in CheckMisc() This variable wasn't being utilized when comments indicate that it probably should be. --- Source/Core/DiscIO/VolumeVerifier.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/DiscIO/VolumeVerifier.cpp b/Source/Core/DiscIO/VolumeVerifier.cpp index 6b70382044..7f339911f9 100644 --- a/Source/Core/DiscIO/VolumeVerifier.cpp +++ b/Source/Core/DiscIO/VolumeVerifier.cpp @@ -601,7 +601,7 @@ void VolumeVerifier::CheckMisc() const Severity severity = m_volume.GetVolumeType() == Platform::WiiWAD ? Severity::Low : Severity::High; // i18n: This is "common" as in "shared", not the opposite of "uncommon" - AddProblem(Severity::Low, GetStringT("This title is set to use an invalid common key.")); + AddProblem(severity, GetStringT("This title is set to use an invalid common key.")); } if (common_key == 1 && region != Region::NTSC_K) From 0ccaa2b5d65234acc10009fa3a8d83071a8ebcac Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 27 May 2019 10:12:20 -0400 Subject: [PATCH 2/6] DiscIO/VolumeVerifier: Take std::string by value in AddProblem() This allows both std::moving into the function and moving the parameter from within the function, potentially avoiding an unnecessary copy. --- Source/Core/DiscIO/VolumeVerifier.cpp | 32 +++++++++++++-------------- Source/Core/DiscIO/VolumeVerifier.h | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Source/Core/DiscIO/VolumeVerifier.cpp b/Source/Core/DiscIO/VolumeVerifier.cpp index 7f339911f9..39061283da 100644 --- a/Source/Core/DiscIO/VolumeVerifier.cpp +++ b/Source/Core/DiscIO/VolumeVerifier.cpp @@ -191,9 +191,9 @@ bool VolumeVerifier::CheckPartition(const Partition& partition) if (m_volume.SupportsIntegrityCheck() && !m_volume.CheckH3TableIntegrity(partition)) { - const std::string text = StringFromFormat( + std::string text = StringFromFormat( GetStringT("The H3 hash table for the %s partition is not correct.").c_str(), name.c_str()); - AddProblem(Severity::Low, text); + AddProblem(Severity::Low, std::move(text)); } bool invalid_disc_header = false; @@ -224,18 +224,18 @@ bool VolumeVerifier::CheckPartition(const Partition& partition) // This can happen when certain programs that create WBFS files scrub the entirety of // the Masterpiece partitions in Super Smash Bros. Brawl without removing them from // the partition table. https://bugs.dolphin-emu.org/issues/8733 - const std::string text = StringFromFormat( + std::string text = StringFromFormat( GetStringT("The %s partition does not seem to contain valid data.").c_str(), name.c_str()); - AddProblem(severity, text); + AddProblem(severity, std::move(text)); return false; } const DiscIO::FileSystem* filesystem = m_volume.GetFileSystem(partition); if (!filesystem) { - const std::string text = StringFromFormat( + std::string text = StringFromFormat( GetStringT("The %s partition does not have a valid file system.").c_str(), name.c_str()); - AddProblem(severity, text); + AddProblem(severity, std::move(text)); return false; } @@ -306,7 +306,7 @@ std::string VolumeVerifier::GetPartitionName(std::optional type) const return name; } -void VolumeVerifier::CheckCorrectlySigned(const Partition& partition, const std::string& error_text) +void VolumeVerifier::CheckCorrectlySigned(const Partition& partition, std::string error_text) { IOS::HLE::Kernel ios; const auto es = ios.GetES(); @@ -321,7 +321,7 @@ void VolumeVerifier::CheckCorrectlySigned(const Partition& partition, const std: IOS::HLE::Device::ES::VerifyMode::DoNotUpdateCertStore, m_volume.GetTMD(partition), cert_chain)) { - AddProblem(Severity::Low, error_text); + AddProblem(Severity::Low, std::move(error_text)); } } @@ -377,7 +377,7 @@ void VolumeVerifier::CheckDiscSize() { const bool second_layer_missing = biggest_offset > SL_DVD_SIZE && m_volume.GetSize() >= SL_DVD_SIZE; - const std::string text = + std::string text = second_layer_missing ? GetStringT( "This disc image is too small and lacks some data. The problem is most likely that " @@ -385,7 +385,7 @@ void VolumeVerifier::CheckDiscSize() GetStringT( "This disc image is too small and lacks some data. If your dumping program saved " "the disc image as several parts, you need to merge them into one file."); - AddProblem(Severity::High, text); + AddProblem(Severity::High, std::move(text)); return; } @@ -819,10 +819,10 @@ void VolumeVerifier::Finish() if (pair.second > 0) { const std::string name = GetPartitionName(m_volume.GetPartitionType(pair.first)); - const std::string text = StringFromFormat( + std::string text = StringFromFormat( GetStringT("Errors were found in %zu blocks in the %s partition.").c_str(), pair.second, name.c_str()); - AddProblem(Severity::Medium, text); + AddProblem(Severity::Medium, std::move(text)); } } @@ -831,10 +831,10 @@ void VolumeVerifier::Finish() if (pair.second > 0) { const std::string name = GetPartitionName(m_volume.GetPartitionType(pair.first)); - const std::string text = StringFromFormat( + std::string text = StringFromFormat( GetStringT("Errors were found in %zu unused blocks in the %s partition.").c_str(), pair.second, name.c_str()); - AddProblem(Severity::Low, text); + AddProblem(Severity::Low, std::move(text)); } } @@ -904,9 +904,9 @@ const VolumeVerifier::Result& VolumeVerifier::GetResult() const return m_result; } -void VolumeVerifier::AddProblem(Severity severity, const std::string& text) +void VolumeVerifier::AddProblem(Severity severity, std::string text) { - m_result.problems.emplace_back(Problem{severity, text}); + m_result.problems.emplace_back(Problem{severity, std::move(text)}); } } // namespace DiscIO diff --git a/Source/Core/DiscIO/VolumeVerifier.h b/Source/Core/DiscIO/VolumeVerifier.h index c0e6072651..e8c68ef26f 100644 --- a/Source/Core/DiscIO/VolumeVerifier.h +++ b/Source/Core/DiscIO/VolumeVerifier.h @@ -90,7 +90,7 @@ private: void CheckPartitions(); bool CheckPartition(const Partition& partition); // Returns false if partition should be ignored std::string GetPartitionName(std::optional type) const; - void CheckCorrectlySigned(const Partition& partition, const std::string& error_text); + void CheckCorrectlySigned(const Partition& partition, std::string error_text); bool IsDebugSigned() const; bool ShouldHaveChannelPartition() const; bool ShouldHaveInstallPartition() const; @@ -103,7 +103,7 @@ private: void SetUpHashing(); bool CheckContentIntegrity(const IOS::ES::Content& content); - void AddProblem(Severity severity, const std::string& text); + void AddProblem(Severity severity, std::string text); const Volume& m_volume; Result m_result; From 52eb2d0d826fddd2c0248ad753ee629419277936 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 27 May 2019 10:19:49 -0400 Subject: [PATCH 3/6] DiscIO/VolumeVerifier: Default destructor within the cpp file Given the volume verifier has quite a few non-trivial object within it, it's best to default the destructor within the cpp file to prevent inlining complex destruction logic elsewhere, while also making it nicer if a forward-declared type is ever used in a member variable. --- Source/Core/DiscIO/VolumeVerifier.cpp | 2 ++ Source/Core/DiscIO/VolumeVerifier.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Source/Core/DiscIO/VolumeVerifier.cpp b/Source/Core/DiscIO/VolumeVerifier.cpp index 39061283da..653cbbaf9c 100644 --- a/Source/Core/DiscIO/VolumeVerifier.cpp +++ b/Source/Core/DiscIO/VolumeVerifier.cpp @@ -54,6 +54,8 @@ VolumeVerifier::VolumeVerifier(const Volume& volume, Hashes hashes_to_calc { } +VolumeVerifier::~VolumeVerifier() = default; + void VolumeVerifier::Start() { ASSERT(!m_started); diff --git a/Source/Core/DiscIO/VolumeVerifier.h b/Source/Core/DiscIO/VolumeVerifier.h index e8c68ef26f..4c2be055d1 100644 --- a/Source/Core/DiscIO/VolumeVerifier.h +++ b/Source/Core/DiscIO/VolumeVerifier.h @@ -72,6 +72,8 @@ public: }; VolumeVerifier(const Volume& volume, Hashes hashes_to_calculate); + ~VolumeVerifier(); + void Start(); void Process(); u64 GetBytesProcessed() const; From fa57396e97302b25204d29dbef132a88cef45c80 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 27 May 2019 10:23:52 -0400 Subject: [PATCH 4/6] DiscIO/VolumeVerifier: In-class initialize members where applicable Removes redundant initializers from the constructor and provides initializers for all members that don't already have one for consistency (and deterministic initial state). --- Source/Core/DiscIO/VolumeVerifier.cpp | 2 +- Source/Core/DiscIO/VolumeVerifier.h | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Source/Core/DiscIO/VolumeVerifier.cpp b/Source/Core/DiscIO/VolumeVerifier.cpp index 653cbbaf9c..5b2b549e6a 100644 --- a/Source/Core/DiscIO/VolumeVerifier.cpp +++ b/Source/Core/DiscIO/VolumeVerifier.cpp @@ -50,7 +50,7 @@ VolumeVerifier::VolumeVerifier(const Volume& volume, Hashes hashes_to_calc : m_volume(volume), m_hashes_to_calculate(hashes_to_calculate), m_calculating_any_hash(hashes_to_calculate.crc32 || hashes_to_calculate.md5 || hashes_to_calculate.sha1), - m_started(false), m_done(false), m_progress(0), m_max_progress(volume.GetSize()) + m_max_progress(volume.GetSize()) { } diff --git a/Source/Core/DiscIO/VolumeVerifier.h b/Source/Core/DiscIO/VolumeVerifier.h index 4c2be055d1..f302891431 100644 --- a/Source/Core/DiscIO/VolumeVerifier.h +++ b/Source/Core/DiscIO/VolumeVerifier.h @@ -109,13 +109,13 @@ private: const Volume& m_volume; Result m_result; - bool m_is_tgc; - bool m_is_datel; - bool m_is_not_retail; + bool m_is_tgc = false; + bool m_is_datel = false; + bool m_is_not_retail = false; - Hashes m_hashes_to_calculate; - bool m_calculating_any_hash; - unsigned long m_crc32_context; + Hashes m_hashes_to_calculate{}; + bool m_calculating_any_hash = false; + unsigned long m_crc32_context = 0; mbedtls_md5_context m_md5_context; mbedtls_sha1_context m_sha1_context; @@ -127,10 +127,10 @@ private: std::map m_block_errors; std::map m_unused_block_errors; - bool m_started; - bool m_done; - u64 m_progress; - u64 m_max_progress; + bool m_started = false; + bool m_done = false; + u64 m_progress = 0; + u64 m_max_progress = 0; }; } // namespace DiscIO From bf6948c1d42ddd7487c41d63d02c10052aaac054 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 27 May 2019 10:28:45 -0400 Subject: [PATCH 5/6] DiscIO/VolumeVerifier: Use structured bindings where applicable Allows providing better names than "first" or "second". --- Source/Core/DiscIO/VolumeVerifier.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Source/Core/DiscIO/VolumeVerifier.cpp b/Source/Core/DiscIO/VolumeVerifier.cpp index 5b2b549e6a..e7d325d918 100644 --- a/Source/Core/DiscIO/VolumeVerifier.cpp +++ b/Source/Core/DiscIO/VolumeVerifier.cpp @@ -816,26 +816,26 @@ void VolumeVerifier::Finish() } } - for (auto pair : m_block_errors) + for (auto [partition, blocks] : m_block_errors) { - if (pair.second > 0) + if (blocks > 0) { - const std::string name = GetPartitionName(m_volume.GetPartitionType(pair.first)); + const std::string name = GetPartitionName(m_volume.GetPartitionType(partition)); std::string text = StringFromFormat( - GetStringT("Errors were found in %zu blocks in the %s partition.").c_str(), pair.second, + GetStringT("Errors were found in %zu blocks in the %s partition.").c_str(), blocks, name.c_str()); AddProblem(Severity::Medium, std::move(text)); } } - for (auto pair : m_unused_block_errors) + for (auto [partition, blocks] : m_unused_block_errors) { - if (pair.second > 0) + if (blocks > 0) { - const std::string name = GetPartitionName(m_volume.GetPartitionType(pair.first)); + const std::string name = GetPartitionName(m_volume.GetPartitionType(partition)); std::string text = StringFromFormat( - GetStringT("Errors were found in %zu unused blocks in the %s partition.").c_str(), - pair.second, name.c_str()); + GetStringT("Errors were found in %zu unused blocks in the %s partition.").c_str(), blocks, + name.c_str()); AddProblem(Severity::Low, std::move(text)); } } From d220e33862c68404efb66f971d778fa6fdb762e1 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 27 May 2019 10:33:02 -0400 Subject: [PATCH 6/6] DiscIO/VolumeVerifier: Make no-argument overload of GetBiggestUsedOffset() const The overload taking a partition is already a const member function, so this one can be turned into one as well. --- Source/Core/DiscIO/VolumeVerifier.cpp | 2 +- Source/Core/DiscIO/VolumeVerifier.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/DiscIO/VolumeVerifier.cpp b/Source/Core/DiscIO/VolumeVerifier.cpp index e7d325d918..9389ae0fbf 100644 --- a/Source/Core/DiscIO/VolumeVerifier.cpp +++ b/Source/Core/DiscIO/VolumeVerifier.cpp @@ -446,7 +446,7 @@ void VolumeVerifier::CheckDiscSize() } } -u64 VolumeVerifier::GetBiggestUsedOffset() +u64 VolumeVerifier::GetBiggestUsedOffset() const { std::vector partitions = m_volume.GetPartitions(); if (partitions.empty()) diff --git a/Source/Core/DiscIO/VolumeVerifier.h b/Source/Core/DiscIO/VolumeVerifier.h index f302891431..31a4e591d9 100644 --- a/Source/Core/DiscIO/VolumeVerifier.h +++ b/Source/Core/DiscIO/VolumeVerifier.h @@ -99,7 +99,7 @@ private: bool ShouldHaveMasterpiecePartitions() const; bool ShouldBeDualLayer() const; void CheckDiscSize(); - u64 GetBiggestUsedOffset(); + u64 GetBiggestUsedOffset() const; u64 GetBiggestUsedOffset(const FileInfo& file_info) const; void CheckMisc(); void SetUpHashing();