Add error handling to the plist XML parser

git-svn-id: https://gtkpod.svn.sf.net/svnroot/gtkpod/libgpod/trunk@2066 f01d2545-417e-4e96-918e-98f8d0dbbcb6

5 files changed, 95 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index acbeb06..daf79aa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2008-07-27  Christophe Fergeau  <teuf@gnome.org>
+
+	* src/itdb_device.c: add itdb_device_error_quark()
+	* src/itdb_plist.h:
+	* src/itdb_plist.c: add error handling to XML parsing (using
+	GError)
+	* src/itdb_sysinfo_extended_parser.c: pass a NULL GError to call to
+	plist parsing function
+
 2008-07-26  Christophe Fergeau  <teuf@gnome.org>
 
 	* tests/test-fw-id.c: (main): add missing call to g_type_init
diff --git a/src/itdb_device.c b/src/itdb_device.c
index 5b29780..7d7819c 100644
--- a/src/itdb_device.c
+++ b/src/itdb_device.c
@@ -1691,3 +1691,12 @@ get_ipod_info_from_serial (const char *serial)
      */
     return g_hash_table_lookup (model_table->serial_hash, serial+len-3);
 }
+
+GQuark itdb_device_error_quark (void)
+{
+    static GQuark quark = 0;
+    if (!quark) {
+        quark = g_quark_from_static_string ("itdb-device-error-quark");
+    }
+    return quark;
+}
diff --git a/src/itdb_plist.c b/src/itdb_plist.c
index b5ee5b8..6efdc48 100644
--- a/src/itdb_plist.c
+++ b/src/itdb_plist.c
@@ -66,7 +66,7 @@
 #define DEBUG(...)
 #endif
 
-static GValue *parse_node (xmlNode *a_node);
+static GValue *parse_node (xmlNode *a_node, GError **error);
 
 static void 
 value_free (GValue *val)
@@ -76,14 +76,21 @@ value_free (GValue *val)
 }
 
 static GValue * 
-parse_integer(xmlNode *a_node)
+parse_integer(xmlNode *a_node, GError **error)
 {
     char *str_val;
+    char *end_ptr;
     gint int_val;
     GValue *value;
 
     str_val = (char *)xmlNodeGetContent(a_node);
-    int_val = strtol (str_val, NULL, 0);
+    int_val = strtol (str_val, &end_ptr, 0);
+    if (*end_ptr != '\0') {
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "invalid integer value: %s", str_val);
+        xmlFree (str_val);
+        return NULL;
+    }
     xmlFree (str_val);
 
     value = g_new0(GValue, 1);
@@ -94,7 +101,7 @@ parse_integer(xmlNode *a_node)
 }
 
 static GValue * 
-parse_string(xmlNode *a_node)
+parse_string(xmlNode *a_node, G_GNUC_UNUSED GError **error)
 {
     char *str_val;
     GValue *value;
@@ -111,14 +118,21 @@ parse_string(xmlNode *a_node)
 }
 
 static GValue *
-parse_real(xmlNode *a_node)
+parse_real(xmlNode *a_node, GError **error)
 {
     char *str_val;
+    char *end_ptr;
     gfloat double_val;
     GValue *value;
 
     str_val = (char *)xmlNodeGetContent(a_node);
-    double_val = g_ascii_strtod (str_val, NULL);
+    double_val = g_ascii_strtod (str_val, &end_ptr);
+    if (*end_ptr != '\0') {
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "invalid real value: %s", str_val);
+        xmlFree (str_val);
+        return NULL;
+    }
     xmlFree (str_val);
 
     value = g_new0(GValue, 1);
@@ -129,7 +143,7 @@ parse_real(xmlNode *a_node)
 }
 
 static GValue *
-parse_boolean (xmlNode *a_node)
+parse_boolean (xmlNode *a_node, GError **error)
 {
     gboolean bool_val;
     GValue *value;
@@ -139,7 +153,8 @@ parse_boolean (xmlNode *a_node)
     } else if (strcmp ((char *)a_node->name, "false") == 0) {
         bool_val = FALSE;
     } else {
-        DEBUG ("unexpected boolean value\n");
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "unexpected boolean value: %s", a_node->name);
         return NULL;
     }
 
@@ -151,7 +166,7 @@ parse_boolean (xmlNode *a_node)
 }
 
 static GValue *
-parse_data (xmlNode *a_node)
+parse_data (xmlNode *a_node, G_GNUC_UNUSED GError **error)
 {
     char *str_val;
     guchar *raw_data;
@@ -173,7 +188,7 @@ parse_data (xmlNode *a_node)
 }
 
 static xmlNode *
-parse_one_dict_entry (xmlNode *a_node, GHashTable *dict)
+parse_one_dict_entry (xmlNode *a_node, GHashTable *dict, GError **error)
 {
     xmlNode *cur_node = a_node;
     xmlChar *key_name;
@@ -186,6 +201,8 @@ parse_one_dict_entry (xmlNode *a_node, GHashTable *dict)
         cur_node = cur_node->next;
     }
     if (cur_node == NULL) {
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "Dict entry contains no <key> node");
         return NULL;
     }
     key_name = xmlNodeGetContent(cur_node);
@@ -194,14 +211,19 @@ parse_one_dict_entry (xmlNode *a_node, GHashTable *dict)
         cur_node = cur_node->next;
     }
     if (cur_node == NULL) {
-        DEBUG ("<key> %s with no corresponding value node", key_name);
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "<key> %s with no corresponding value node", key_name);
         xmlFree (key_name);
         return NULL;
     }
 
-    value = parse_node (cur_node);
+    value = parse_node (cur_node, error);
     if (value != NULL) {
         g_hash_table_insert (dict, g_strdup ((char *)key_name), value);
+    } else {
+        g_warning ("Couldn't parse value for %s: %s",
+                   key_name, (*error)->message);
+        g_clear_error (error);
     }
     xmlFree (key_name);
 
@@ -209,7 +231,7 @@ parse_one_dict_entry (xmlNode *a_node, GHashTable *dict)
 }
 
 static GValue * 
-parse_dict (xmlNode *a_node)
+parse_dict (xmlNode *a_node, GError **error)
 {
     xmlNode *cur_node = a_node->children;
     GValue *value;
@@ -219,9 +241,11 @@ parse_dict (xmlNode *a_node)
                                   g_free, (GDestroyNotify)value_free);
 
     while (cur_node != NULL) {
-        cur_node = parse_one_dict_entry (cur_node, dict);
+        cur_node = parse_one_dict_entry (cur_node, dict, error);
+    }
+    if ((error != NULL) && (*error != NULL)) {
+        return NULL;
     }
-
     value = g_new0 (GValue, 1);
     value = g_value_init (value, G_TYPE_HASH_TABLE);
     g_value_take_boxed (value, dict);
@@ -229,7 +253,7 @@ parse_dict (xmlNode *a_node)
     return value;
 }
 
-typedef GValue *(*ParseCallback) (xmlNode *);
+typedef GValue *(*ParseCallback) (xmlNode *, GError **);
 struct Parser {
     const char * const type_name;
     ParseCallback parser;
@@ -245,14 +269,14 @@ static struct Parser parsers[] = { {"integer", parse_integer},
                                    {"array",   parse_dict},
                                    {NULL,      NULL} };
 
-static GValue *parse_node (xmlNode *a_node)
+static GValue *parse_node (xmlNode *a_node, GError **error)
 {
     guint i = 0;
     g_return_val_if_fail (a_node != NULL, FALSE);
     while (parsers[i].type_name != NULL) {        
         if (xmlStrcmp (a_node->name, (xmlChar *)parsers[i].type_name) == 0) {
             if (parsers[i].parser != NULL) {
-                return parsers[i].parser (a_node);
+                return parsers[i].parser (a_node, error);
             }
         }
         i++;
@@ -262,15 +286,17 @@ static GValue *parse_node (xmlNode *a_node)
 }
 
 static GValue *
-itdb_plist_parse (xmlNode * a_node)
+itdb_plist_parse (xmlNode * a_node, GError **error)
 {
     xmlNode *cur_node;
     if (a_node == NULL) {
-        DEBUG ("empty file\n");
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "Empty XML document");
         return NULL;
     }
     if (xmlStrcmp (a_node->name, (xmlChar *)"plist") != 0) {
-        DEBUG ("not a plist file\n");
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "XML document does not seem to be a plist document");
         return NULL;
     }
     cur_node = a_node->xmlChildrenNode;
@@ -278,13 +304,15 @@ itdb_plist_parse (xmlNode * a_node)
         cur_node = cur_node->next;
     }
     if (cur_node != NULL) {
-        return parse_node (cur_node);
+        return parse_node (cur_node, error);
     }
+    g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                 "Empty XML document");
     return NULL;
 }
 
 GValue *
-itdb_plist_parse_from_file (const char *filename)
+itdb_plist_parse_from_file (const char *filename, GError **error)
 {
     xmlDoc *doc = NULL;
     xmlNode *root_element = NULL;
@@ -293,13 +321,14 @@ itdb_plist_parse_from_file (const char *filename)
     doc = xmlReadFile(filename, NULL, 0);
 
     if (doc == NULL) {
-        printf("error: could not parse file %s\n", filename);
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "Error during XML parsing of file %s", filename);
         return NULL;
     }
 
     root_element = xmlDocGetRootElement(doc);
 
-    parsed_doc = itdb_plist_parse (root_element);
+    parsed_doc = itdb_plist_parse (root_element, error);
 
     xmlFreeDoc(doc);
     xmlCleanupParser();
@@ -308,7 +337,7 @@ itdb_plist_parse_from_file (const char *filename)
 }
 
 GValue *
-itdb_plist_parse_from_memory (const char *data, gsize len)
+itdb_plist_parse_from_memory (const char *data, gsize len, GError **error)
 {
     xmlDoc *doc = NULL;
     xmlNode *root_element = NULL;
@@ -317,13 +346,14 @@ itdb_plist_parse_from_memory (const char *data, gsize len)
     doc = xmlReadMemory(data, len, "noname.xml", NULL, 0);
 
     if (doc == NULL) {
-        printf("error: could not parse data from memory\n");
+        g_set_error (error, ITDB_DEVICE_ERROR, ITDB_DEVICE_ERROR_XML_PARSING,
+                     "Error during XML parsing of in-memory data");
         return NULL;
     }
 
     root_element = xmlDocGetRootElement(doc);
 
-    parsed_doc = itdb_plist_parse (root_element);
+    parsed_doc = itdb_plist_parse (root_element, error);
 
     xmlFreeDoc(doc);
     xmlCleanupParser();
@@ -334,13 +364,15 @@ itdb_plist_parse_from_memory (const char *data, gsize len)
 #include <glib-object.h>
 #include "itdb_plist.h"
 
-GValue *itdb_plist_parse_from_file (G_GNUC_UNUSED const char *filename)
+GValue *itdb_plist_parse_from_file (G_GNUC_UNUSED const char *filename,
+                                    G_GNUC_UNUSED GError **error)
 {
     return NULL;
 }
 
 GValue *itdb_plist_parse_from_memory (G_GNUC_UNUSED const char *data, 
-                                      G_GNUC_UNUSED gsize len)
+                                      G_GNUC_UNUSED gsize len,
+                                      G_GNUC_UNUSED GError **error)
 {
     return NULL;
 }
diff --git a/src/itdb_plist.h b/src/itdb_plist.h
index 50b5618..976c2ba 100644
--- a/src/itdb_plist.h
+++ b/src/itdb_plist.h
@@ -33,9 +33,21 @@
 
 G_BEGIN_DECLS
 
-G_GNUC_INTERNAL GValue *itdb_plist_parse_from_file (const char *filename);
+typedef enum {
+	ITDB_DEVICE_ERROR_XML_PARSING
+} ItdbDeviceError;
+
+/* Error domain */
+#define ITDB_DEVICE_ERROR itdb_device_error_quark ()
+GQuark     itdb_device_error_quark      (void);
+
+
+
+G_GNUC_INTERNAL GValue *itdb_plist_parse_from_file (const char *filename,
+						    GError **error);
 G_GNUC_INTERNAL GValue *itdb_plist_parse_from_memory (const char *data, 
-                                                      gsize len); 
+                                                      gsize len,
+						      GError **error); 
 
 G_END_DECLS
 
diff --git a/src/itdb_sysinfo_extended_parser.c b/src/itdb_sysinfo_extended_parser.c
index 8352682..324e42d 100644
--- a/src/itdb_sysinfo_extended_parser.c
+++ b/src/itdb_sysinfo_extended_parser.c
@@ -507,7 +507,7 @@ SysInfoIpodProperties *itdb_sysinfo_extended_parse (const char *filename)
 
     g_return_val_if_fail (filename != NULL, NULL);
 
-    parsed_doc = itdb_plist_parse_from_file (filename);
+    parsed_doc = itdb_plist_parse_from_file (filename, NULL);
     if (parsed_doc == NULL) {
         return NULL;
     }