From f96a0ce9d2e8e33fe9ccce2c1d077a7ec3b0ac33 Mon Sep 17 00:00:00 2001 From: Megamouse Date: Sun, 28 May 2023 13:36:45 +0200 Subject: [PATCH] Trophies: Add more sanity checks to pugixml backend --- Utilities/rXml.cpp | 89 +++++++++++++++++-------- Utilities/rXml.h | 4 +- rpcs3/Emu/Cell/Modules/sceNpTrophy.cpp | 38 +++++++---- rpcs3/Loader/TROPUSR.cpp | 17 ++++- rpcs3/rpcs3qt/trophy_manager_dialog.cpp | 37 ++++++---- 5 files changed, 129 insertions(+), 56 deletions(-) diff --git a/Utilities/rXml.cpp b/Utilities/rXml.cpp index 6f223fa4d7..db52fb38e3 100644 --- a/Utilities/rXml.cpp +++ b/Utilities/rXml.cpp @@ -5,54 +5,81 @@ rXmlNode::rXmlNode() { } -rXmlNode::rXmlNode(const pugi::xml_node &node) +rXmlNode::rXmlNode(const pugi::xml_node& node) { handle = node; } std::shared_ptr rXmlNode::GetChildren() { - // it.begin() returns node_iterator*, *it.begin() return node*. - pugi::xml_object_range it = handle.children(); - pugi::xml_node begin = *it.begin(); + if (handle) + { + if (const pugi::xml_node child = handle.first_child()) + { + return std::make_shared(child); + } + } - if (begin) - { - return std::make_shared(begin); - } - else - { - return nullptr; - } + return nullptr; } std::shared_ptr rXmlNode::GetNext() { - pugi::xml_node result = handle.next_sibling(); - if (result) + if (handle) { - return std::make_shared(result); - } - else - { - return nullptr; + if (const pugi::xml_node result = handle.next_sibling()) + { + return std::make_shared(result); + } } + + return nullptr; } std::string rXmlNode::GetName() { - return handle.name(); + if (handle) + { + if (const pugi::char_t* name = handle.name()) + { + return name; + } + } + + return {}; } -std::string rXmlNode::GetAttribute(const std::string &name) +std::string rXmlNode::GetAttribute(const std::string& name) { - auto pred = [&name](pugi::xml_attribute attr) { return (name == attr.name()); }; - return handle.find_attribute(pred).value(); + if (handle) + { + const auto pred = [&name](const pugi::xml_attribute& attr) { return (name == attr.name()); }; + if (const pugi::xml_attribute attr = handle.find_attribute(pred)) + { + if (const pugi::char_t* value = attr.value()) + { + return value; + } + } + } + + return {}; } std::string rXmlNode::GetNodeContent() { - return handle.text().get(); + if (handle) + { + if (const pugi::xml_text text = handle.text()) + { + if (const pugi::char_t* value = text.get()) + { + return value; + } + } + } + + return {}; } rXmlDocument::rXmlDocument() @@ -61,10 +88,20 @@ rXmlDocument::rXmlDocument() pugi::xml_parse_result rXmlDocument::Read(const std::string& data) { - return handle.load_buffer(data.data(), data.size()); + if (handle) + { + return handle.load_buffer(data.data(), data.size()); + } + + return {}; } std::shared_ptr rXmlDocument::GetRoot() { - return std::make_shared(handle.root()); + if (const pugi::xml_node root = handle.root()) + { + return std::make_shared(root); + } + + return nullptr; } diff --git a/Utilities/rXml.h b/Utilities/rXml.h index 4d8f378130..71ca257902 100644 --- a/Utilities/rXml.h +++ b/Utilities/rXml.h @@ -19,11 +19,11 @@ struct rXmlNode { rXmlNode(); - rXmlNode(const pugi::xml_node &); + rXmlNode(const pugi::xml_node& node); std::shared_ptr GetChildren(); std::shared_ptr GetNext(); std::string GetName(); - std::string GetAttribute(const std::string &name); + std::string GetAttribute(const std::string& name); std::string GetNodeContent(); pugi::xml_node handle{}; diff --git a/rpcs3/Emu/Cell/Modules/sceNpTrophy.cpp b/rpcs3/Emu/Cell/Modules/sceNpTrophy.cpp index c24e0d43db..497b8cd32b 100644 --- a/rpcs3/Emu/Cell/Modules/sceNpTrophy.cpp +++ b/rpcs3/Emu/Cell/Modules/sceNpTrophy.cpp @@ -873,21 +873,27 @@ error_code sceNpTrophyGetGameInfo(u32 context, u32 handle, vm::ptr trophy_base = doc.GetRoot(); + if (!trophy_base) + { + sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM (root is null): %s", config_path); + // TODO: return some error + return CELL_OK; + } for (std::shared_ptr n = trophy_base->GetChildren(); n; n = n->GetNext()) { @@ -1156,10 +1162,14 @@ static error_code NpTrophyGetTrophyInfo(const trophy_context_t* ctxt, s32 trophy } auto trophy_base = doc.GetRoot(); - ensure(trophy_base); + if (!trophy_base) + { + sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM (root is null): %s", config_path); + // TODO: return some error + } bool found = false; - for (std::shared_ptr n = trophy_base->GetChildren(); n; n = n->GetNext()) + for (std::shared_ptr n = trophy_base ? trophy_base->GetChildren() : nullptr; n; n = n->GetNext()) { if (n->GetName() == "trophy" && (trophyId == atoi(n->GetAttribute("id").c_str()))) { @@ -1401,14 +1411,18 @@ error_code sceNpTrophyGetTrophyIcon(u32 context, u32 handle, s32 trophyId, vm::p pugi::xml_parse_result res = doc.Read(config.to_string()); if (!res) { - sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM: %s", config_path); + sceNpTrophy.error("sceNpTrophyGetTrophyIcon: Failed to read TROPCONF.SFM: %s", config_path); // TODO: return some error } auto trophy_base = doc.GetRoot(); - ensure(trophy_base); + if (!trophy_base) + { + sceNpTrophy.error("sceNpTrophyGetTrophyIcon: Failed to read TROPCONF.SFM (root is null): %s", config_path); + // TODO: return some error + } - for (std::shared_ptr n = trophy_base->GetChildren(); n; n = n->GetNext()) + for (std::shared_ptr n = trophy_base ? trophy_base->GetChildren() : nullptr; n; n = n->GetNext()) { if (n->GetName() == "trophy" && trophyId == atoi(n->GetAttribute("id").c_str()) && n->GetAttribute("hidden")[0] == 'y') { diff --git a/rpcs3/Loader/TROPUSR.cpp b/rpcs3/Loader/TROPUSR.cpp index 590c534daf..67b84961fd 100644 --- a/rpcs3/Loader/TROPUSR.cpp +++ b/rpcs3/Loader/TROPUSR.cpp @@ -12,7 +12,10 @@ enum : u32 std::shared_ptr trophy_xml_document::GetRoot() { auto trophy_base = rXmlDocument::GetRoot(); - ensure(trophy_base); + if (!trophy_base) + { + return nullptr; + } if (auto trophy_conf = trophy_base->GetChildren(); trophy_conf && trophy_conf->GetName() == "trophyconf") @@ -178,7 +181,11 @@ bool TROPUSRLoader::Generate(const std::string& filepath, const std::string& con m_table6.clear(); auto trophy_base = doc.GetRoot(); - ensure(trophy_base); + if (!trophy_base) + { + trp_log.error("TROPUSRLoader::Generate: Failed to read file (root is null): %s", filepath); + return false; + } for (std::shared_ptr n = trophy_base->GetChildren(); n; n = n->GetNext()) { @@ -276,7 +283,11 @@ u32 TROPUSRLoader::GetUnlockedPlatinumID(u32 trophy_id, const std::string& confi } auto trophy_base = doc.GetRoot(); - ensure(trophy_base); + if (!trophy_base) + { + trp_log.error("TROPUSRLoader::GetUnlockedPlatinumID: Failed to read file (root is null): %s", config_path); + return false; + } const usz trophy_count = m_table4.size(); diff --git a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp index 97b9c7f3dc..aa070c900e 100644 --- a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp +++ b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp @@ -461,7 +461,11 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name) } std::shared_ptr trophy_base = game_trophy_data->trop_config.GetRoot(); - ensure(trophy_base); + if (!trophy_base) + { + gui_log.error("Failed to read trophy xml (root is null): %s", tropconf_path); + return false; + } for (std::shared_ptr n = trophy_base->GetChildren(); n; n = n->GetNext()) { @@ -914,7 +918,7 @@ void trophy_manager_dialog::StartTrophyLoadThreads() } const QDir trophy_dir(trophy_path); - const auto folder_list = trophy_dir.entryList(QDir::Dirs | QDir::NoDotAndDotDot); + const QStringList folder_list = trophy_dir.entryList(QDir::Dirs | QDir::NoDotAndDotDot); const int count = folder_list.count(); if (count <= 0) @@ -941,22 +945,28 @@ void trophy_manager_dialog::StartTrophyLoadThreads() close(); // It's pointless to show an empty window }); - futureWatcher.setFuture(QtConcurrent::map(indices, [this, folder_list](const int& i) + atomic_t error_count{}; + futureWatcher.setFuture(QtConcurrent::map(indices, [this, &error_count, &folder_list](const int& i) { const std::string dir_name = sstr(folder_list.value(i)); gui_log.trace("Loading trophy dir: %s", dir_name); if (!LoadTrophyFolderToDB(dir_name)) { - // TODO: Add error checks & throws to LoadTrophyFolderToDB so that they can be caught here. - // Also add a way of showing the number of corrupted/invalid folders in UI somewhere. + // TODO: add a way of showing the number of corrupted/invalid folders in UI somewhere. gui_log.error("Error occurred while parsing folder %s for trophies.", dir_name); + error_count++; } })); progressDialog.exec(); futureWatcher.waitForFinished(); + + if (error_count != 0) + { + gui_log.error("Failed to load %d of %d trophy folders!", error_count.load(), count); + } } void trophy_manager_dialog::PopulateGameTable() @@ -1028,16 +1038,19 @@ void trophy_manager_dialog::PopulateTrophyTable() m_trophy_table->setRowCount(all_trophies); m_trophy_table->setSortingEnabled(false); // Disable sorting before using setItem calls - std::shared_ptr trophy_base = data->trop_config.GetRoot(); - ensure(trophy_base); - QPixmap placeholder(m_icon_height, m_icon_height); placeholder.fill(Qt::transparent); const QLocale locale{}; + std::shared_ptr trophy_base = data->trop_config.GetRoot(); + if (!trophy_base) + { + gui_log.error("Populating Trophy Manager UI failed (root is null): %s %s", data->game_name, data->path); + } + int i = 0; - for (std::shared_ptr n = trophy_base->GetChildren(); n; n = n->GetNext()) + for (std::shared_ptr n = trophy_base ? trophy_base->GetChildren() : nullptr; n; n = n->GetNext()) { // Only show trophies. if (n->GetName() != "trophy") @@ -1077,13 +1090,11 @@ void trophy_manager_dialog::PopulateTrophyTable() { if (n2->GetName() == "name") { - std::string name = n2->GetNodeContent(); - strcpy_trunc(details.name, name); + strcpy_trunc(details.name, n2->GetNodeContent()); } if (n2->GetName() == "detail") { - std::string detail = n2->GetNodeContent(); - strcpy_trunc(details.description, detail); + strcpy_trunc(details.description, n2->GetNodeContent()); } }