Add better XML error logging and do some related cleanup

Error details from libxml2 are no longer ignored and are properly
logged now. XML initialization code is now in the XML namespace.
The XML::Document constructor that took a data pointer was removed
because it wasn't being used and it would make the new logging less
useful (no filename).

Signed-off-by: Chuck Miller

3 files changed, 54 insertions, 34 deletions

diff --git a/src/main.cpp b/src/main.cpp
index 56019976..29713c27 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -21,11 +21,10 @@
 
 #include "main.h"
 
-#include "utils/gettext.h"
-
 #include "client.h"
 
-#include <libxml/parser.h>
+#include "utils/gettext.h"
+#include "utils/xml.h"
 
 #include <getopt.h>
 #include <iostream>
@@ -178,22 +177,6 @@ static void initInternationalization()
 #endif
 }
 
-static void xmlNullLogger(void *ctx, const char *msg, ...)
-{
-    // Does nothing, that's the whole point of it
-}
-
-// Initialize libxml2 and check for potential ABI mismatches between
-// compiled version and the shared library actually used.
-static void initXML()
-{
-    xmlInitParser();
-    LIBXML_TEST_VERSION;
-
-    // Suppress libxml2 error messages
-    xmlSetGenericErrorFunc(NULL, xmlNullLogger);
-}
-
 
 int main(int argc, char *argv[])
 {
@@ -227,7 +210,7 @@ int main(int argc, char *argv[])
     }
     atexit((void(*)()) PHYSFS_deinit);
 
-    initXML();
+    XML::init();
 
     Client client(options);
     return client.exec();
diff --git a/src/utils/xml.cpp b/src/utils/xml.cpp
index 341f34c0..8d444fab 100644
--- a/src/utils/xml.cpp
+++ b/src/utils/xml.cpp
@@ -25,14 +25,30 @@
 
 #include "resources/resourcemanager.h"
 
-#include <iostream>
+#include <libxml/parser.h>
+#include <libxml/xmlerror.h>
+
 #include <fstream>
+#include <iostream>
 
 namespace XML
 {
+    static void xmlLogger(void *ctx, xmlErrorPtr error);
+
+    struct XMLContext
+    {
+        std::string file;
+        bool resman;
+    };
+
     Document::Document(const std::string &filename, bool useResman):
         mDoc(0)
     {
+        XMLContext *ctx = new XMLContext();
+        ctx->file = filename;
+        ctx->resman = useResman;
+        xmlSetStructuredErrorFunc(ctx, xmlLogger);
+
         int size;
         char *data = NULL;
         if (useResman)
@@ -75,11 +91,8 @@ namespace XML
         {
             logger->log("Error loading %s", filename.c_str());
         }
-    }
 
-    Document::Document(const char *data, int size)
-    {
-        mDoc = xmlParseMemory(data, size);
+        xmlSetStructuredErrorFunc(NULL, xmlLogger);
     }
 
     Document::~Document()
@@ -144,4 +157,32 @@ namespace XML
         return NULL;
     }
 
+    static void xmlLogger(void *ctx, xmlErrorPtr error)
+    {
+        XMLContext *context = static_cast<XMLContext*>(ctx);
+
+        if (context)
+            logger->log("Error in XML file '%s' on line %d",
+                        context->file.c_str(), error->line);
+        else
+            logger->log("Error in unknown xml file on line %d",
+                        error->line);
+
+        logger->log(error->message);
+
+        // No need to keep errors around
+        xmlCtxtResetLastError(error->ctxt);
+    }
+
+    void init()
+    {
+        // Initialize libxml2 and check for potential ABI mismatches between
+        // compiled version and the shared library actually used.
+        xmlInitParser();
+        LIBXML_TEST_VERSION;
+
+        // Handle libxml2 error messages
+        xmlSetStructuredErrorFunc(NULL, xmlLogger);
+    }
+
 } // namespace XML
diff --git a/src/utils/xml.h b/src/utils/xml.h
index 72745501..8ffecb76 100644
--- a/src/utils/xml.h
+++ b/src/utils/xml.h
@@ -45,15 +45,6 @@ namespace XML
             Document(const std::string &filename, bool useResman = true);
 
             /**
-             * Constructor that attempts to load an XML document from memory.
-             * Does not log errors.
-             *
-             * @param data the string to parse as XML
-             * @param size the length of the string in bytes
-             */
-            Document(const char *data, int size);
-
-            /**
              * Destructor. Frees the loaded XML file.
              */
             ~Document();
@@ -88,6 +79,11 @@ namespace XML
      * Finds the first child node with the given name
      */
     xmlNodePtr findFirstChildByName(xmlNodePtr parent, const char *name);
+
+    /**
+     * Initializes the XML engine.
+     */
+    void init();
 }
 
 #define for_each_xml_child_node(var, parent) \