Skip to content

Commit

Permalink
Merge pull request #11951 from Minty-Meeo/dolphin-tool-code-review-1
Browse files Browse the repository at this point in the history 
DolphinTool: Sensible changes
  • Loading branch information
@AdmiralCurtiss
AdmiralCurtiss committed Jun 17, 2023
2 parents cbcf5bf + 31e78c4 commit 98b5d72
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 88 deletions.
7 changes: 2 additions & 5 deletions Source/Core/DolphinTool/ConvertCommand.cpp
View file
Expand Up @@ -340,8 +340,7 @@ ConvertCommand::ParseCompressionTypeString(const std::string& compression_str)
return DiscIO::WIARVZCompressionType::LZMA2;
else if (compression_str == "zstd")
return DiscIO::WIARVZCompressionType::Zstd;
else
return std::nullopt;
return std::nullopt;
}

std::optional<DiscIO::BlobType> ConvertCommand::ParseFormatString(const std::string& format_str)
Expand All @@ -354,8 +353,6 @@ std::optional<DiscIO::BlobType> ConvertCommand::ParseFormatString(const std::str
return DiscIO::BlobType::WIA;
else if (format_str == "rvz")
return DiscIO::BlobType::RVZ;
else
return std::nullopt;
return std::nullopt;
}

} // namespace DolphinTool
25 changes: 12 additions & 13 deletions Source/Core/DolphinTool/HeaderCommand.cpp
View file
Expand Up @@ -13,29 +13,29 @@ namespace DolphinTool
{
int HeaderCommand::Main(const std::vector<std::string>& args)
{
auto parser = std::make_unique<optparse::OptionParser>();
optparse::OptionParser parser;

parser->usage("usage: header [options]...");
parser.usage("usage: header [options]...");

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("-b", "--block_size")
parser.add_option("-b", "--block_size")
.action("store_true")
.help("Optional. Print the block size of GCZ/WIA/RVZ formats, then exit.");

parser->add_option("-c", "--compression")
parser.add_option("-c", "--compression")
.action("store_true")
.help("Optional. Print the compression method of GCZ/WIA/RVZ formats, then exit.");

parser->add_option("-l", "--compression_level")
parser.add_option("-l", "--compression_level")
.action("store_true")
.help("Optional. Print the level of compression for WIA/RVZ formats, then exit.");

const optparse::Values& options = parser->parse_args(args);
const optparse::Values& options = parser.parse_args(args);

// Validate options
const std::string input_file_path = static_cast<const char*>(options.get("input"));
Expand All @@ -45,18 +45,17 @@ int HeaderCommand::Main(const std::vector<std::string>& args)
return 1;
}

bool enable_block_size = options.is_set_by_user("block_size");
bool enable_compression_method = options.is_set_by_user("compression");
bool enable_compression_level = options.is_set_by_user("compression_level");
const bool enable_block_size = options.is_set_by_user("block_size");
const bool enable_compression_method = options.is_set_by_user("compression");
const bool enable_compression_level = options.is_set_by_user("compression_level");

// Open the blob reader, plus get blob type
std::shared_ptr<DiscIO::BlobReader> blob_reader = DiscIO::CreateBlobReader(input_file_path);
const std::unique_ptr<DiscIO::BlobReader> blob_reader = DiscIO::CreateBlobReader(input_file_path);
if (!blob_reader)
{
std::cerr << "Error: Unable to open disc image" << std::endl;
return 1;
}
const DiscIO::BlobType blob_type = blob_reader->GetBlobType();

if (enable_block_size || enable_compression_method || enable_compression_level)
{
Expand Down Expand Up @@ -87,6 +86,7 @@ int HeaderCommand::Main(const std::vector<std::string>& args)
}
else
{
const auto blob_type = blob_reader->GetBlobType();
if (blob_type == DiscIO::BlobType::GCZ)
{
std::cout << "Block Size: " << blob_reader->GetBlockSize() << std::endl;
Expand All @@ -102,5 +102,4 @@ int HeaderCommand::Main(const std::vector<std::string>& args)

return 0;
}

} // namespace DolphinTool
10 changes: 4 additions & 6 deletions Source/Core/DolphinTool/ToolMain.cpp
View file
Expand Up @@ -5,6 +5,7 @@
#include <cstring>
#include <iostream>
#include <string>
#include <string_view>
#include <vector>

#include "Common/Version.h"
Expand Down Expand Up @@ -33,12 +34,9 @@ int main(int argc, char* argv[])
if (argc < 2)
return PrintUsage(1);

std::vector<std::string> args(argv, argv + argc);

std::string command_str = args.at(1);

// Take off the command selector before passing arguments down
args.erase(args.begin(), args.begin() + 1);
const std::string_view command_str = argv[1];
// Take off the program name and command selector before passing arguments down
const std::vector<std::string> args(argv + 2, argv + argc);

std::unique_ptr<DolphinTool::Command> command;

Expand Down
92 changes: 33 additions & 59 deletions Source/Core/DolphinTool/VerifyCommand.cpp
View file
Expand Up @@ -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
Expand All @@ -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")
Expand All @@ -80,34 +76,36 @@ 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;
return 1;
}

// 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;
Expand All @@ -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)
{
Expand All @@ -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;
Expand All @@ -197,5 +172,4 @@ std::string VerifyCommand::HashToHexString(const std::vector<u8>& hash)
}
return ss.str();
}

} // namespace DolphinTool
6 changes: 1 addition & 5 deletions Source/Core/DolphinTool/VerifyCommand.h
View file
Expand Up @@ -20,11 +20,7 @@ class VerifyCommand final : public Command
int Main(const std::vector<std::string>& args) override;

private:
void PrintFullReport(const std::optional<DiscIO::VolumeVerifier::Result> result);

std::optional<DiscIO::VolumeVerifier::Result>
VerifyVolume(std::shared_ptr<DiscIO::VolumeDisc> volume,
const DiscIO::Hashes<bool>& hashes_to_calculate);
void PrintFullReport(const DiscIO::VolumeVerifier::Result& result);

std::string HashToHexString(const std::vector<u8>& hash);
};
Expand Down

0 comments on commit 98b5d72

Please sign in to comment.