Merge pull request #11951 from Minty-Meeo/dolphin-tool-code-review-1

DolphinTool: Sensible changes

1 files changed, 33 insertions, 59 deletions

diff --git a/Source/Core/DolphinTool/VerifyCommand.cpp b/Source/Core/DolphinTool/VerifyCommand.cpp
index f046025c12..631ac229f4 100644
--- a/Source/Core/DolphinTool/VerifyCommand.cpp
+++ b/Source/Core/DolphinTool/VerifyCommand.cpp
@@ -10,29 +10,29 @@ namespace DolphinTool
 {
 int VerifyCommand::Main(const std::vector<std::string>& args)
 {
-  auto parser = std::make_unique<optparse::OptionParser>();
+  optparse::OptionParser parser;
 
-  parser->usage("usage: verify [options]...");
+  parser.usage("usage: verify [options]...");
 
-  parser->add_option("-u", "--user")
+  parser.add_option("-u", "--user")
       .action("store")
       .help("User folder path, required for temporary processing files. "
             "Will be automatically created if this option is not set.");
 
-  parser->add_option("-i", "--input")
+  parser.add_option("-i", "--input")
       .type("string")
       .action("store")
       .help("Path to disc image FILE.")
       .metavar("FILE");
 
-  parser->add_option("-a", "--algorithm")
+  parser.add_option("-a", "--algorithm")
       .type("string")
       .action("store")
       .help("Optional. Compute and print the digest using the selected algorithm, then exit. "
             "[%choices]")
       .choices({"crc32", "md5", "sha1"});
 
-  const optparse::Values& options = parser->parse_args(args);
+  const optparse::Values& options = parser.parse_args(args);
 
   // Initialize the dolphin user directory, required for temporary processing files
   // If this is not set, destructive file operations could occur due to path confusion
@@ -51,19 +51,15 @@ int VerifyCommand::Main(const std::vector<std::string>& args)
     return 1;
   }
 
-  std::optional<std::string> algorithm;
-  if (options.is_set("algorithm"))
-  {
-    algorithm = static_cast<const char*>(options.get("algorithm"));
-  }
-
   DiscIO::Hashes<bool> hashes_to_calculate{};
-  if (algorithm == std::nullopt)
+  const bool algorithm_is_set = options.is_set("algorithm");
+  if (!algorithm_is_set)
   {
     hashes_to_calculate = DiscIO::VolumeVerifier::GetDefaultHashesToCalculate();
   }
   else
   {
+    const std::string algorithm = static_cast<const char*>(options.get("algorithm"));
     if (algorithm == "crc32")
       hashes_to_calculate.crc32 = true;
     else if (algorithm == "md5")
@@ -80,7 +76,7 @@ int VerifyCommand::Main(const std::vector<std::string>& args)
   }
 
   // Open the volume
-  std::shared_ptr<DiscIO::VolumeDisc> volume = DiscIO::CreateDisc(input_file_path);
+  const std::unique_ptr<DiscIO::VolumeDisc> volume = DiscIO::CreateDisc(input_file_path);
   if (!volume)
   {
     std::cerr << "Error: Unable to open disc image" << std::endl;
@@ -88,26 +84,28 @@ int VerifyCommand::Main(const std::vector<std::string>& args)
   }
 
   // Verify the volume
-  const std::optional<DiscIO::VolumeVerifier::Result> result =
-      VerifyVolume(volume, hashes_to_calculate);
-  if (!result)
+  DiscIO::VolumeVerifier verifier(*volume, false, hashes_to_calculate);
+  verifier.Start();
+  while (verifier.GetBytesProcessed() != verifier.GetTotalBytes())
   {
-    std::cerr << "Error: Unable to verify volume" << std::endl;
-    return 1;
+    verifier.Process();
   }
+  verifier.Finish();
+  const DiscIO::VolumeVerifier::Result& result = verifier.GetResult();
 
-  if (algorithm == std::nullopt)
+  // Print the report
+  if (!algorithm_is_set)
   {
     PrintFullReport(result);
   }
   else
   {
-    if (hashes_to_calculate.crc32 && !result->hashes.crc32.empty())
-      std::cout << HashToHexString(result->hashes.crc32) << std::endl;
-    else if (hashes_to_calculate.md5 && !result->hashes.md5.empty())
-      std::cout << HashToHexString(result->hashes.md5) << std::endl;
-    else if (hashes_to_calculate.sha1 && !result->hashes.sha1.empty())
-      std::cout << HashToHexString(result->hashes.sha1) << std::endl;
+    if (hashes_to_calculate.crc32 && !result.hashes.crc32.empty())
+      std::cout << HashToHexString(result.hashes.crc32) << std::endl;
+    else if (hashes_to_calculate.md5 && !result.hashes.md5.empty())
+      std::cout << HashToHexString(result.hashes.md5) << std::endl;
+    else if (hashes_to_calculate.sha1 && !result.hashes.sha1.empty())
+      std::cout << HashToHexString(result.hashes.sha1) << std::endl;
     else
     {
       std::cerr << "Error: No hash computed" << std::endl;
@@ -118,29 +116,27 @@ int VerifyCommand::Main(const std::vector<std::string>& args)
   return 0;
 }
 
-void VerifyCommand::PrintFullReport(const std::optional<DiscIO::VolumeVerifier::Result> result)
+void VerifyCommand::PrintFullReport(const DiscIO::VolumeVerifier::Result& result)
 {
-  if (!result->hashes.crc32.empty())
-    std::cout << "CRC32: " << HashToHexString(result->hashes.crc32) << std::endl;
+  if (!result.hashes.crc32.empty())
+    std::cout << "CRC32: " << HashToHexString(result.hashes.crc32) << std::endl;
   else
     std::cout << "CRC32 not computed" << std::endl;
 
-  if (!result->hashes.md5.empty())
-    std::cout << "MD5: " << HashToHexString(result->hashes.md5) << std::endl;
+  if (!result.hashes.md5.empty())
+    std::cout << "MD5: " << HashToHexString(result.hashes.md5) << std::endl;
   else
     std::cout << "MD5 not computed" << std::endl;
 
-  if (!result->hashes.sha1.empty())
-    std::cout << "SHA1: " << HashToHexString(result->hashes.sha1) << std::endl;
+  if (!result.hashes.sha1.empty())
+    std::cout << "SHA1: " << HashToHexString(result.hashes.sha1) << std::endl;
   else
     std::cout << "SHA1 not computed" << std::endl;
 
-  std::cout << "Problems Found: " << (result->problems.size() > 0 ? "Yes" : "No") << std::endl;
+  std::cout << "Problems Found: " << (result.problems.empty() ? "No" : "Yes") << std::endl;
 
-  for (int i = 0; i < static_cast<int>(result->problems.size()); ++i)
+  for (const auto& problem : result.problems)
   {
-    const DiscIO::VolumeVerifier::Problem problem = result->problems[i];
-
     std::cout << std::endl << "Severity: ";
     switch (problem.severity)
     {
@@ -166,27 +162,6 @@ void VerifyCommand::PrintFullReport(const std::optional<DiscIO::VolumeVerifier::
   }
 }
 
-std::optional<DiscIO::VolumeVerifier::Result>
-VerifyCommand::VerifyVolume(std::shared_ptr<DiscIO::VolumeDisc> volume,
-                            const DiscIO::Hashes<bool>& hashes_to_calculate)
-{
-  if (!volume)
-    return std::nullopt;
-
-  DiscIO::VolumeVerifier verifier(*volume, false, hashes_to_calculate);
-
-  verifier.Start();
-  while (verifier.GetBytesProcessed() != verifier.GetTotalBytes())
-  {
-    verifier.Process();
-  }
-  verifier.Finish();
-
-  const DiscIO::VolumeVerifier::Result result = verifier.GetResult();
-
-  return result;
-}
-
 std::string VerifyCommand::HashToHexString(const std::vector<u8>& hash)
 {
   std::stringstream ss;
@@ -197,5 +172,4 @@ std::string VerifyCommand::HashToHexString(const std::vector<u8>& hash)
   }
   return ss.str();
 }
-
 }  // namespace DolphinTool