From d88329fe7b27ccf6a1cdae97d020a12f51d253ad Mon Sep 17 00:00:00 2001
From: Bent Bisballe Nyeng <deva@aasimon.org>
Date: Wed, 25 Jul 2018 19:54:32 +0200
Subject: Rewrite ConfigParser to use pugixml.

---
 plugin/drumgizmo_plugin.cc |  2 +-
 src/configparser.cc        | 42 +++++++++++++++++++-----------------------
 src/configparser.h         | 10 ++--------
 test/Makefile.am           |  5 ++---
 test/configparsertest.cc   | 23 ++++++++++++++++++++---
 5 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/plugin/drumgizmo_plugin.cc b/plugin/drumgizmo_plugin.cc
index a5dd7be..af2198c 100644
--- a/plugin/drumgizmo_plugin.cc
+++ b/plugin/drumgizmo_plugin.cc
@@ -595,7 +595,7 @@ bool DrumGizmoPlugin::ConfigStringIO::set(std::string config_string)
 	DEBUG(config, "Load config: %s\n", config_string.c_str());
 
 	ConfigParser p;
-	if(p.parseString(config_string))
+	if(!p.parseString(config_string))
 	{
 		ERR(config, "Config parse error.\n");
 		return false;
diff --git a/src/configparser.cc b/src/configparser.cc
index ef657b0..0227d15 100644
--- a/src/configparser.cc
+++ b/src/configparser.cc
@@ -27,37 +27,33 @@
 #include "configparser.h"
 
 #include <hugin.hpp>
+#include <pugixml.hpp>
 
-#include "saxparser.h"
-
-ConfigParser::ConfigParser()
-{
-	str = nullptr;
-}
-
-void ConfigParser::characterData(const std::string& data)
+bool ConfigParser::parseString(const std::string& xml)
 {
-	if(str)
+	pugi::xml_document doc;
+	pugi::xml_parse_result result = doc.load_buffer(xml.data(), xml.size());
+	if(result.status)
 	{
-		str->append(data);
+		ERR(configparser, "XML parse error: %d", (int)result.offset);
+		return false;
 	}
-}
 
-void ConfigParser::startTag(const std::string& name, const attr_t& attr)
-{
-	if(name == "value" && attr.find("name") != attr.end())
-	{
-		values[attr.at("name")] = "";
-		str = &values[attr.at("name")];
-	}
-}
+	//TODO: handle xml version
 
-void ConfigParser::endTag(const std::string& name)
-{
-	if(name == "value")
+	pugi::xml_node config_node = doc.child("config");
+	for(pugi::xml_node value_node : config_node.children("value"))
 	{
-		str = nullptr;
+		auto name = value_node.attribute("name").as_string();
+		if(std::string(name) == "")
+		{
+			continue;
+		}
+		auto value = value_node.child_value();
+		values[name] = value;
 	}
+
+	return true;
 }
 
 std::string ConfigParser::value(const std::string& name, const std::string& def)
diff --git a/src/configparser.h b/src/configparser.h
index 29e47b0..71b33eb 100644
--- a/src/configparser.h
+++ b/src/configparser.h
@@ -28,20 +28,14 @@
 
 #include <unordered_map>
 
-#include "saxparser.h"
-
 class ConfigParser
-	: public SAXParser
 {
 public:
-	ConfigParser();
+	//! Returns false on failure, true on success.
+	bool parseString(const std::string& xml);
 
-	void characterData(const std::string& data) override;
-	void startTag(const std::string& name, const attr_t& attr) override;
-	void endTag(const std::string& name) override;
 	std::string value(const std::string& name, const std::string& def = "");
 
 private:
 	std::unordered_map<std::string, std::string> values;
-	std::string* str;
 };
diff --git a/test/Makefile.am b/test/Makefile.am
index c26b017..2a34275 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -219,12 +219,11 @@ domloadertest_SOURCES = \
 
 configparsertest_CXXFLAGS = -DOUTPUT=\"configparsertest\" \
 	-I$(top_srcdir)/src -I$(top_srcdir)/include -I$(top_srcdir)/hugin \
-	-I$(top_srcdir)/pugixml/src $(EXPAT_CFLAGS)
-configparsertest_LDFLAGS = $(EXPAT_LIBS)
+	-I$(top_srcdir)/pugixml/src
+configparsertest_LDFLAGS =
 configparsertest_SOURCES = \
 	$(top_srcdir)/hugin/hugin.c \
 	$(top_srcdir)/src/configparser.cc \
-	$(top_srcdir)/src/saxparser.cc \
 	$(top_srcdir)/pugixml/src/pugixml.cpp \
 	configparsertest.cc \
 	dgtest.cc
diff --git a/test/configparsertest.cc b/test/configparsertest.cc
index de2f37e..0b00d0c 100644
--- a/test/configparsertest.cc
+++ b/test/configparsertest.cc
@@ -35,6 +35,7 @@ public:
 	ConfigParserTest()
 	{
 		DGUNIT_TEST(ConfigParserTest::test);
+		DGUNIT_TEST(ConfigParserTest::invalid);
 	}
 
 	void test()
@@ -44,20 +45,36 @@ public:
 			"<config>\n" \
 			"  <value name=\"foo\">42</value>\n" \
 			"  <value name=\"bar\">true</value>\n" \
-			"  <value name=\"bas\">\"</value>\n" \
+			"  <value name=\"bas\">&quot;&lt;</value>\n" \
 			"</config>";
 
 
 		ConfigParser parser;
-		DGUNIT_ASSERT(!parser.parseString(xml));
+		DGUNIT_ASSERT(parser.parseString(xml));
 
 		DGUNIT_ASSERT_EQUAL(std::string("42"), parser.value("foo", "-"));
 		DGUNIT_ASSERT_EQUAL(std::string("true"), parser.value("bar", "-"));
-		DGUNIT_ASSERT_EQUAL(std::string("\""), parser.value("bas", "-"));
+		DGUNIT_ASSERT_EQUAL(std::string("\"<"), parser.value("bas", "-"));
 
 		// Non-existing value
 		DGUNIT_ASSERT_EQUAL(std::string("-"), parser.value("bas2", "-"));
 	}
+
+	void invalid()
+	{
+		std::string xml =
+			"<?xml version='1.0' encoding='UTF-8'?>\n" \
+			"<config\n" \
+			"  <value name=\"foo\">42</value>\n" \
+			"  <value name=\"bar\">true</value>\n" \
+			"  <value name=\"bas\">&quot;&lt;</value>\n" \
+			"</config>";
+
+
+		ConfigParser parser;
+		// Epxect parser error (missing '>' in line 2)
+		DGUNIT_ASSERT(!parser.parseString(xml));
+	}
 };
 
 // Registers the fixture into the 'registry'
-- 
cgit v1.2.3