From bf978ac8cab89f53b2065cbe3b8455ea0aed3b3f Mon Sep 17 00:00:00 2001 From: Megamouse Date: Fri, 19 Jun 2020 15:38:51 +0200 Subject: [PATCH] patch manager: properly check patch versions Also abort patch import of lower patch versions --- Utilities/bin_patch.cpp | 43 +++++++++++----------- Utilities/bin_patch.h | 4 +-- Utilities/version.cpp | 49 ++++++++++++++++++++++++++ Utilities/version.h | 3 ++ rpcs3/rpcs3qt/patch_manager_dialog.cpp | 4 ++- 5 files changed, 80 insertions(+), 23 deletions(-) diff --git a/Utilities/bin_patch.cpp b/Utilities/bin_patch.cpp index 873892f8e9..989e3a3b40 100644 --- a/Utilities/bin_patch.cpp +++ b/Utilities/bin_patch.cpp @@ -1,6 +1,7 @@ #include "bin_patch.h" #include "File.h" #include "Config.h" +#include "version.h" LOG_CHANNEL(patch_log); @@ -639,7 +640,7 @@ void patch_engine::save_config(const patch_map& patches_map, bool enable_legacy_ file.write(out.c_str(), out.size()); } -static void append_patches(patch_engine::patch_map& existing_patches, const patch_engine::patch_map& new_patches) +static bool append_patches(patch_engine::patch_map& existing_patches, const patch_engine::patch_map& new_patches, std::stringstream* log_messages) { for (const auto& [hash, new_container] : new_patches) { @@ -661,24 +662,21 @@ static void append_patches(patch_engine::patch_map& existing_patches, const patc auto& info = container.patch_info_map[description]; - const auto version_is_bigger = [](const std::string& v0, const std::string& v1, const std::string& hash, const std::string& description) + bool ok; + const bool version_is_bigger = utils::compare_versions(new_info.patch_version, info.patch_version, ok) > 0; + + if (!ok) { - std::add_pointer_t ev0, ev1; - const double ver0 = std::strtod(v0.c_str(), &ev0); - const double ver1 = std::strtod(v1.c_str(), &ev1); - - if (v0.c_str() + v0.size() == ev0 && v1.c_str() + v1.size() == ev1) - { - return ver0 > ver1; - } - - patch_log.error("Failed to compare patch versions ('%s' vs '%s') for %s: %s", v0, v1, hash, description); + patch_log.error("Failed to compare patch versions ('%s' vs '%s') for %s: %s", new_info.patch_version, info.patch_version, hash, description); + append_log_message(log_messages, fmt::format("Failed to compare patch versions ('%s' vs '%s') for %s: %s", new_info.patch_version, info.patch_version, hash, description)); return false; - }; + } - if (!version_is_bigger(new_info.patch_version, info.patch_version, hash, description)) + if (!version_is_bigger) { - continue; + patch_log.error("A higher or equal patch version already exists ('%s' vs '%s') for %s: %s", new_info.patch_version, info.patch_version, hash, description); + append_log_message(log_messages, fmt::format("A higher or equal patch version already exists ('%s' vs '%s') for %s: %s", new_info.patch_version, info.patch_version, hash, description)); + return false; } if (!new_info.patch_version.empty()) info.patch_version = new_info.patch_version; @@ -689,14 +687,17 @@ static void append_patches(patch_engine::patch_map& existing_patches, const patc if (!new_info.data_list.empty()) info.data_list = new_info.data_list; } } + + return true; } -bool patch_engine::save_patches(const patch_map& patches, const std::string& path) +bool patch_engine::save_patches(const patch_map& patches, const std::string& path, std::stringstream* log_messages) { fs::file file(path, fs::rewrite); if (!file) { patch_log.fatal("save_patches: Failed to open patch file %s", path); + append_log_message(log_messages, fmt::format("Failed to open patch file %s", path)); return false; } @@ -755,14 +756,16 @@ bool patch_engine::save_patches(const patch_map& patches, const std::string& pat return true; } -bool patch_engine::import_patches(const patch_engine::patch_map& patches, const std::string& path) +bool patch_engine::import_patches(const patch_engine::patch_map& patches, const std::string& path, std::stringstream* log_messages) { patch_engine::patch_map existing_patches; - if (load(existing_patches, path, true)) + if (load(existing_patches, path, true, log_messages)) { - append_patches(existing_patches, patches); - return save_patches(existing_patches, path); + if (append_patches(existing_patches, patches, log_messages)) + { + return save_patches(existing_patches, path, log_messages); + } } return false; diff --git a/Utilities/bin_patch.h b/Utilities/bin_patch.h index b926fdc4a4..b5d6a958c4 100644 --- a/Utilities/bin_patch.h +++ b/Utilities/bin_patch.h @@ -102,10 +102,10 @@ public: static void save_config(const patch_map& patches_map, bool enable_legacy_patches); // Save a patch file - static bool save_patches(const patch_map& patches, const std::string& path); + static bool save_patches(const patch_map& patches, const std::string& path, std::stringstream* log_messages = nullptr); // Create or append patches to a file - static bool import_patches(const patch_map& patches, const std::string& path); + static bool import_patches(const patch_map& patches, const std::string& path, std::stringstream* log_messages = nullptr); // Load patch_config.yml static patch_config_map load_config(bool& enable_legacy_patches); diff --git a/Utilities/version.cpp b/Utilities/version.cpp index f1262f31c1..d067b7feef 100644 --- a/Utilities/version.cpp +++ b/Utilities/version.cpp @@ -1,6 +1,8 @@ #include "stdafx.h" #include "version.h" +#include + namespace utils { std::string to_string(version_type type) @@ -48,4 +50,51 @@ namespace utils return version; } + + // Based on https://www.geeksforgeeks.org/compare-two-version-numbers/ + int compare_versions(const std::string& v1, const std::string& v2, bool& ok) + { + // Check if both version strings are valid + ok = std::regex_match(v1, std::regex("[0-9.]*")) && std::regex_match(v2, std::regex("[0-9.]*")); + + if (!ok) + { + return -1; + } + + // vnum stores each numeric part of version + int vnum1 = 0; + int vnum2 = 0; + + // Loop until both strings are processed + for (int i = 0, j = 0; (i < v1.length() || j < v2.length());) + { + // Storing numeric part of version 1 in vnum1 + while (i < v1.length() && v1[i] != '.') + { + vnum1 = vnum1 * 10 + (v1[i] - '0'); + i++; + } + + // Storing numeric part of version 2 in vnum2 + while (j < v2.length() && v2[j] != '.') + { + vnum2 = vnum2 * 10 + (v2[j] - '0'); + j++; + } + + if (vnum1 > vnum2) + return 1; + + if (vnum2 > vnum1) + return -1; + + // If equal, reset variables and go for next numeric part + vnum1 = vnum2 = 0; + i++; + j++; + } + + return 0; + } } diff --git a/Utilities/version.h b/Utilities/version.h index da66a675d4..24389c1098 100644 --- a/Utilities/version.h +++ b/Utilities/version.h @@ -69,4 +69,7 @@ namespace utils uint to_hex() const; std::string to_string() const; }; + + // Generic version comparison (e.g. 0.0.5 vs 1.3) + int compare_versions(const std::string& v1, const std::string& v2, bool& ok); } diff --git a/rpcs3/rpcs3qt/patch_manager_dialog.cpp b/rpcs3/rpcs3qt/patch_manager_dialog.cpp index 7c4422a05a..34b73afe27 100644 --- a/rpcs3/rpcs3qt/patch_manager_dialog.cpp +++ b/rpcs3/rpcs3qt/patch_manager_dialog.cpp @@ -424,7 +424,9 @@ void patch_manager_dialog::dropEvent(QDropEvent* event) { static const std::string imported_patch_yml_path = fs::get_config_dir() + "patches/imported_patch.yml"; - if (patch_engine::import_patches(patches, imported_patch_yml_path)) + log_message.clear(); + + if (patch_engine::import_patches(patches, imported_patch_yml_path, &log_message)) { refresh(); QMessageBox::information(this, tr("Import successful"), tr("The patch file was imported to:\n%0").arg(QString::fromStdString(imported_patch_yml_path)));