[virt-tools-list] [PATCH 28/47] Pass a GError all the way through the XML parser

Daniel P. Berrange berrange at redhat.com
Wed Aug 25 19:37:23 UTC 2010


When reading the XML files, a GError should be passed
all the way into the parser functions to enable more
useful error reporting

* osinfo/osinfo_dataread.c: Pass GError down into
  all XML parser APIs
* osinfo/osinfo_db.c, osinfo/osinfo_db.h: Remove
  return value, since this can be detected from
  the GError status
* test/test-skeleton.c: Pass in a GError & report
  on failures
---
 osinfo/osinfo_dataread.c |   95 ++++++++++++++++++++--------------------------
 osinfo/osinfo_db.c       |   15 ++-----
 osinfo/osinfo_db.h       |    2 +-
 test/test-skeleton.c     |    7 ++-
 4 files changed, 51 insertions(+), 68 deletions(-)

diff --git a/osinfo/osinfo_dataread.c b/osinfo/osinfo_dataread.c
index b10227d..2618313 100644
--- a/osinfo/osinfo_dataread.c
+++ b/osinfo/osinfo_dataread.c
@@ -16,6 +16,8 @@
 #define WHITESPACE_NODE 14
 #define COMMENT_NODE 8
 
+void osinfo_dataread(OsinfoDb *db, GError **err);
+
 /*
  * TODO:
  * 1. More robust handling of files that are in bad format
@@ -34,41 +36,39 @@
 
 struct __osinfoDbRet {
     OsinfoDb *db;
-    int *ret;
+    GError **err;
 };
 
+#define OSINFO_ERROR(err, msg) \
+  g_set_error_literal((err), g_quark_from_static_string("libosinfo"), 0, (msg));
+
 static gboolean __osinfoResolveDeviceLink(gpointer key, gpointer value, gpointer data)
 {
     gchar *id = (gchar *) key;
     struct __osinfoDeviceLink *devLink = (struct __osinfoDeviceLink *) value;
     struct __osinfoDbRet *dbRet = (struct __osinfoDbRet *) data;
     OsinfoDb *db = dbRet->db;
-    int *ret = dbRet->ret;
     OsinfoDeviceList *devices = osinfo_db_get_device_list(db);
 
     OsinfoDevice *dev = OSINFO_DEVICE(osinfo_list_find_by_id(OSINFO_LIST(devices), id));
     if (!dev) {
-        *ret = -EINVAL;
-        return TRUE;
+        OSINFO_ERROR(dbRet->err, "missing device");
+	return TRUE;
     }
 
     devLink->dev = dev;
-    *ret = 0;
     return FALSE;
 }
 
 static gboolean __osinfoResolveSectionDevices(gpointer key, gpointer value, gpointer data)
 {
+    g_return_val_if_fail(value != NULL, TRUE);
+
     struct __osinfoDbRet *dbRet = (struct __osinfoDbRet *) data;
-    int *ret = dbRet->ret;
-    GTree *section = (GTree *) value;
-    if (!section) {
-        *ret = -EINVAL;
-        return TRUE;
-    }
+    GTree *section = value;
 
     g_tree_foreach(section, __osinfoResolveDeviceLink, dbRet);
-    if (*ret)
+    if (*dbRet->err)
         return TRUE;
     return FALSE;
 }
@@ -78,23 +78,21 @@ static void __osinfoResolveHvLink(gpointer key, gpointer value, gpointer data)
     gchar *hvId = (gchar *) key;
     struct __osinfoDbRet *dbRet = (struct __osinfoDbRet *) data;
     OsinfoDb *db = dbRet->db;
-    int *ret = dbRet->ret;
     struct __osinfoHvSection *hvSection = (struct __osinfoHvSection *) value;
     OsinfoHypervisor *hv;
     OsinfoHypervisorList *hypervisors = osinfo_db_get_hypervisor_list(db);
 
     g_tree_foreach(hvSection->sections, __osinfoResolveSectionDevices, dbRet);
-    if (*ret)
+    if (*dbRet->err)
         return;
 
     hv = OSINFO_HYPERVISOR(osinfo_list_find_by_id(OSINFO_LIST(hypervisors), hvId));
     if (!hv) {
-        *ret = -EINVAL;
+        OSINFO_ERROR(dbRet->err, "missing hypervisor");
         return;
     }
 
     hvSection->hv = hv;
-    *ret = 0;
 }
 
 static gboolean __osinfoResolveOsLink(gpointer key, gpointer value, gpointer data)
@@ -102,81 +100,68 @@ static gboolean __osinfoResolveOsLink(gpointer key, gpointer value, gpointer dat
     gchar *targetOsId = (gchar *) key;
     struct __osinfoDbRet *dbRet = (struct __osinfoDbRet *) data;
     OsinfoDb *db = dbRet->db;
-    int *ret = dbRet->ret;
     struct __osinfoOsLink *osLink = (struct __osinfoOsLink *) value;
     OsinfoOs *targetOs;
     OsinfoOsList *oslist = osinfo_db_get_os_list(db);
 
     targetOs = OSINFO_OS(osinfo_list_find_by_id(OSINFO_LIST(oslist), targetOsId));
     if (!targetOs) {
-        *ret = -EINVAL;
+        OSINFO_ERROR(dbRet->err, "missing os");
         return TRUE;
     }
 
     osLink->directObjectOs = targetOs;
-    *ret = 0;
     return FALSE;
 }
 
 static gboolean __osinfoFixOsLinks(OsinfoList *list, OsinfoEntity *entity, gpointer data)
 {
+    g_return_val_if_fail(OSINFO_OS(entity), TRUE);
+
     struct __osinfoDbRet *dbRet = data;
-    int *ret = dbRet->ret;
     OsinfoOs *os = OSINFO_OS(entity);
-    if (!os) {
-        *ret = -EINVAL;
-        return TRUE;
-    }
 
     g_tree_foreach(os->priv->sections, __osinfoResolveSectionDevices, dbRet);
-    if (*ret)
+    if (*dbRet->err)
         return TRUE;
 
     g_tree_foreach(os->priv->relationshipsByOs, __osinfoResolveOsLink, dbRet);
-    if (*ret)
+    if (*dbRet->err)
         return TRUE;
 
     g_hash_table_foreach(os->priv->hypervisors, __osinfoResolveHvLink, dbRet);
-    if (*ret)
+    if (*dbRet->err)
         return TRUE;
 
-    *ret = 0;
     return FALSE;
 }
 
 static gboolean __osinfoFixHvLinks(OsinfoList *list, OsinfoEntity *entity, gpointer data)
 {
+    g_return_val_if_fail(OSINFO_HYPERVISOR(entity), TRUE);
+
     struct __osinfoDbRet *dbRet = data;
-    int *ret = dbRet->ret;
     OsinfoHypervisor *hv = OSINFO_HYPERVISOR(entity);
-    if (!hv) {
-        *ret = -EINVAL;
-        return TRUE;
-    }
 
     g_tree_foreach(hv->priv->sections, __osinfoResolveSectionDevices, dbRet);
-    if (*ret)
+    if (*dbRet->err)
         return TRUE;
     return FALSE;
 }
 
-static int __osinfoFixObjLinks(OsinfoDb *db)
+static void __osinfoFixObjLinks(OsinfoDb *db, GError **err)
 {
-    int ret;
-
-    if (!OSINFO_IS_DB(db))
-        return -EINVAL;
+    g_return_if_fail(OSINFO_IS_DB(db));
 
-    struct __osinfoDbRet dbRet = {db, &ret};
+    struct __osinfoDbRet dbRet = {db, err };
     OsinfoHypervisorList *hypervisors = osinfo_db_get_hypervisor_list(db);
     OsinfoOsList *oses = osinfo_db_get_os_list(db);
 
     osinfo_list_foreach(OSINFO_LIST(hypervisors), __osinfoFixHvLinks, &dbRet);
-    if (ret)
-        return ret;
-    osinfo_list_foreach(OSINFO_LIST(oses), __osinfoFixOsLinks, &dbRet);
+    if (*dbRet.err)
+        return;
 
-    return ret;
+    osinfo_list_foreach(OSINFO_LIST(oses), __osinfoFixOsLinks, &dbRet);
 }
 
 static int __osinfoProcessTag(xmlTextReaderPtr reader, char** ptr_to_key, char** ptr_to_val)
@@ -831,9 +816,10 @@ cleanup_error:
     return err;
 }
 
-static int __osinfoReadDataFile(OsinfoDb *db,
+static int osinfo_dataread_file(OsinfoDb *db,
                                 const char *dir,
-                                const char *filename)
+                                const char *filename,
+				GError **err)
 {
     int ret;
     xmlTextReaderPtr reader;
@@ -853,7 +839,7 @@ static int __osinfoReadDataFile(OsinfoDb *db,
     return ret;
 }
 
-int __osinfoInitializeData(OsinfoDb *db)
+void osinfo_dataread(OsinfoDb *db, GError **err)
 {
     int ret;
     DIR* dir;
@@ -872,30 +858,31 @@ int __osinfoInitializeData(OsinfoDb *db)
     /* Get XML files in directory */
     dir = opendir(backingDir);
     if (!dir) {
-        ret = errno;
+        g_set_error_literal(err, g_quark_from_static_string("libosinfo"), 0,
+			    "unable to read backing dir");
         goto cleanup;
     }
 
     while ((dp=readdir(dir)) != NULL) {
         if (dp->d_type != DT_REG)
             continue;
-        ret = __osinfoReadDataFile(db, backingDir, dp->d_name);
+        ret = osinfo_dataread_file(db, backingDir, dp->d_name, err);
         if (ret != 0)
             break;
     }
     closedir(dir);
-    if (ret == 0)
-        ret = __osinfoFixObjLinks(db);
+    if (!*err)
+        __osinfoFixObjLinks(db, err);
 
 cleanup:
     xmlCleanupParser();
     g_free(backingDir);
-    return ret;
 }
 
 #else
-int __osinfoInitializeData(OsinfoDb *db)
+void osinfo_dataread(OsinfoDb *db, GError **err)
 {
-    return -ENOSYS;
+    g_set_error_literal(err, g_quark_from_static_string("libosinfo"), 0,
+			"xml loading not available");
 }
 #endif
diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
index 0bee6c1..2a5740f 100644
--- a/osinfo/osinfo_db.c
+++ b/osinfo/osinfo_db.c
@@ -156,18 +156,13 @@ OsinfoDb *osinfo_db_new(const gchar *backingDir)
 }
 
 
-int osinfo_db_initialize(OsinfoDb *self, GError **err)
+extern void osinfo_dataread(OsinfoDb *db, GError **err);
+
+void osinfo_db_initialize(OsinfoDb *self, GError **err)
 {
-    int ret;
-    // And now read in data.
-    ret = __osinfoInitializeData(self);
-    if (ret != 0) {
-        self->priv->ready = 0;
-        g_set_error_literal(err, g_quark_from_static_string("libosinfo"), ret, "Error during reading data");
-    }
-    else
+    osinfo_dataread(self, err);
+    if (!*err)
         self->priv->ready = 1;
-    return ret;
 }
 
 OsinfoHypervisor *osinfo_db_get_hypervisor(OsinfoDb *self, gchar *id)
diff --git a/osinfo/osinfo_db.h b/osinfo/osinfo_db.h
index 55f4620..28d9265 100644
--- a/osinfo/osinfo_db.h
+++ b/osinfo/osinfo_db.h
@@ -62,7 +62,7 @@ GType osinfo_db_get_type(void);
 
 OsinfoDb *osinfo_db_new(const gchar *backingDir);
 
-int osinfo_db_initialize(OsinfoDb *self, GError **err);
+void osinfo_db_initialize(OsinfoDb *self, GError **err);
 
 OsinfoHypervisor *osinfo_db_get_hypervisor(OsinfoDb *self, gchar *hvId);
 OsinfoDevice *osinfo_db_get_device(OsinfoDb *self, gchar *devId);
diff --git a/test/test-skeleton.c b/test/test-skeleton.c
index 016fa20..e0054d4 100644
--- a/test/test-skeleton.c
+++ b/test/test-skeleton.c
@@ -14,11 +14,12 @@ main (int argc, char *argv[])
 
     /* Create our object */
     OsinfoDb *db = osinfo_db_new("../data");
+    GError *err = NULL;
 
     // Read in data
-    ret = osinfo_db_initialize(db, NULL);
-    if (ret != 0) {
-        printf("Error initializing db! %d\n", ret);
+    osinfo_db_initialize(db, &err);
+    if (err) {
+        printf("Error loading db: %s\n", err->message);
         exit(1);
     }
 
-- 
1.7.2.1




More information about the virt-tools-list mailing list