Trophies: Add more sanity checks to pugixml backend

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> rXmlNode::GetChildren()
 {
-	// it.begin() returns node_iterator*, *it.begin() return node*.
+	if (handle)
-	pugi::xml_object_range<pugi::xml_node_iterator> it = handle.children();
+	{
-	pugi::xml_node begin = *it.begin();
+		if (const pugi::xml_node child = handle.first_child())
+		{
+			return std::make_shared<rXmlNode>(child);
+		}
+	}
 
-	if (begin)
+	return nullptr;
-	{
-		return std::make_shared<rXmlNode>(begin);
-	}
-	else
-	{
-		return nullptr;
-	}
 }
 
 std::shared_ptr<rXmlNode> rXmlNode::GetNext()
 {
-	pugi::xml_node result = handle.next_sibling();
+	if (handle)
-	if (result)
 	{
-		return std::make_shared<rXmlNode>(result);
+		if (const pugi::xml_node result = handle.next_sibling())
-	}
+		{
-	else
+			return std::make_shared<rXmlNode>(result);
-	{
+		}
-		return nullptr;
 	}
+
+	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()); };
+	if (handle)
-	return handle.find_attribute(pred).value();
+	{
+		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<rXmlNode> rXmlDocument::GetRoot()
 {
-	return std::make_shared<rXmlNode>(handle.root());
+	if (const pugi::xml_node root = handle.root())
+	{
+		return std::make_shared<rXmlNode>(root);
+	}
+
+	return nullptr;
 }

Utilities/rXml.h

@@ -19,11 +19,11 @@
 struct rXmlNode
 {
 	rXmlNode();
-	rXmlNode(const pugi::xml_node &);
+	rXmlNode(const pugi::xml_node& node);
 	std::shared_ptr<rXmlNode> GetChildren();
 	std::shared_ptr<rXmlNode> 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{};

rpcs3/Emu/Cell/Modules/sceNpTrophy.cpp

@@ -873,21 +873,27 @@ error_code sceNpTrophyGetGameInfo(u32 context, u32 handle, vm::ptr<SceNpTrophyGa
 		return { SCE_NP_TROPHY_ERROR_CONF_DOES_NOT_EXIST, config_path };
 	}
 
+	if (details)
+		*details = {};
+	if (data)
+		*data = {};
+
 	trophy_xml_document doc{};
 	pugi::xml_parse_result res = doc.Read(config.to_string());
 	if (!res)
 	{
 		sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM: %s", config_path);
 		// TODO: return some error
+		return CELL_OK;
 	}
 
-	auto trophy_base = doc.GetRoot();
+	std::shared_ptr<rXmlNode> trophy_base = doc.GetRoot();
-	ensure(trophy_base);
+	if (!trophy_base)
-
+	{
-	if (details)
+		sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM (root is null): %s", config_path);
-		*details = {};
+		// TODO: return some error
-	if (data)
+		return CELL_OK;
-		*data = {};
+	}
 
 	for (std::shared_ptr<rXmlNode> 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<rXmlNode> n = trophy_base->GetChildren(); n; n = n->GetNext())
+	for (std::shared_ptr<rXmlNode> 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<rXmlNode> n = trophy_base->GetChildren(); n; n = n->GetNext())
+			for (std::shared_ptr<rXmlNode> 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')
 				{

rpcs3/Loader/TROPUSR.cpp

@@ -12,7 +12,10 @@ enum : u32
 std::shared_ptr<rXmlNode> 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<rXmlNode> 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();
 

rpcs3/rpcs3qt/trophy_manager_dialog.cpp

@@ -461,7 +461,11 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name)
 	}
 
 	std::shared_ptr<rXmlNode> 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<rXmlNode> 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<usz> 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.
+			// TODO: add a way of showing the number of corrupted/invalid folders in UI somewhere.
-			// Also 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<rXmlNode> 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<rXmlNode> 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<rXmlNode> n = trophy_base->GetChildren(); n; n = n->GetNext())
+	for (std::shared_ptr<rXmlNode> 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, n2->GetNodeContent());
-				strcpy_trunc(details.name, name);
 			}
 			if (n2->GetName() == "detail")
 			{
-				std::string detail = n2->GetNodeContent();
+				strcpy_trunc(details.description, n2->GetNodeContent());
-				strcpy_trunc(details.description, detail);
 			}
 		}