[Libosinfo] [PATCH 2/5] loader: identify referenced but not defined entities

Daniel P. Berrange berrange at redhat.com
Tue Oct 6 16:41:00 UTC 2015


When loading the database we often have to instantiate
entities to represent relationships (upgrades, derives,
etc) before the entity is actually defined. This makes
the code liable to bugs as we might instantiate entities
which are never defined.

This extends the loader so that it tracks all entities
that are created via references, and once loading is
complete it prints out a warning if any referenced
entities were not defined fully.

This identifies a number of mistakes in our current
database that the following patches will fix.

Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
---
 osinfo/osinfo_loader.c | 94 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 75 insertions(+), 19 deletions(-)

diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
index 0c7ddfb..4251a90 100644
--- a/osinfo/osinfo_loader.c
+++ b/osinfo/osinfo_loader.c
@@ -56,6 +56,7 @@ struct _OsinfoLoaderPrivate
 {
     OsinfoDb *db;
     GHashTable *xpath_cache;
+    GHashTable *entityRefs;
 };
 
 struct _OsinfoEntityKey
@@ -73,6 +74,8 @@ osinfo_loader_finalize(GObject *object)
     g_object_unref(loader->priv->db);
     g_hash_table_destroy(loader->priv->xpath_cache);
 
+    g_hash_table_destroy(loader->priv->entityRefs);
+
     /* Chain up to the parent class */
     G_OBJECT_CLASS(osinfo_loader_parent_class)->finalize(object);
 }
@@ -106,6 +109,10 @@ osinfo_loader_init(OsinfoLoader *loader)
                                                       g_str_equal,
                                                       g_free,
                                                       xpath_cache_value_free);
+    loader->priv->entityRefs = g_hash_table_new_full(g_str_hash,
+                                                     g_str_equal,
+                                                     g_free,
+                                                     NULL);
 }
 
 /** PUBLIC METHODS */
@@ -409,61 +416,101 @@ static void osinfo_loader_entity(OsinfoLoader *loader,
 }
 
 static OsinfoDatamap *osinfo_loader_get_datamap(OsinfoLoader *loader,
-                                                 const gchar *id)
+                                                const gchar *id,
+                                                gboolean reference)
 {
     OsinfoDatamap *datamap = osinfo_db_get_datamap(loader->priv->db, id);
     if (!datamap) {
         datamap = osinfo_datamap_new(id);
         osinfo_db_add_datamap(loader->priv->db, datamap);
         g_object_unref(datamap);
+        if (reference) {
+            g_hash_table_insert(loader->priv->entityRefs, g_strdup(id), datamap);
+        }
+    } else {
+        if (!reference) {
+            g_hash_table_remove(loader->priv->entityRefs, id);
+        }
     }
     return datamap;
 }
 
 static OsinfoDevice *osinfo_loader_get_device(OsinfoLoader *loader,
-                                              const gchar *id)
+                                              const gchar *id,
+                                              gboolean reference)
 {
     OsinfoDevice *dev = osinfo_db_get_device(loader->priv->db, id);
     if (!dev) {
         dev = osinfo_device_new(id);
         osinfo_db_add_device(loader->priv->db, dev);
         g_object_unref(dev);
+        if (reference) {
+            g_hash_table_insert(loader->priv->entityRefs, g_strdup(id), dev);
+        }
+    } else {
+        if (!reference) {
+            g_hash_table_remove(loader->priv->entityRefs, id);
+        }
     }
     return dev;
 }
 
 static OsinfoOs *osinfo_loader_get_os(OsinfoLoader *loader,
-                                      const gchar *id)
+                                      const gchar *id,
+                                      gboolean reference)
 {
     OsinfoOs *os = osinfo_db_get_os(loader->priv->db, id);
     if (!os) {
         os = osinfo_os_new(id);
         osinfo_db_add_os(loader->priv->db, os);
         g_object_unref(os);
+        if (reference) {
+            g_hash_table_insert(loader->priv->entityRefs, g_strdup(id), os);
+        }
+    } else {
+        if (!reference) {
+            g_hash_table_remove(loader->priv->entityRefs, id);
+        }
     }
     return os;
 }
 
 static OsinfoPlatform *osinfo_loader_get_platform(OsinfoLoader *loader,
-                                                  const gchar *id)
+                                                  const gchar *id,
+                                                  gboolean reference)
 {
     OsinfoPlatform *platform = osinfo_db_get_platform(loader->priv->db, id);
     if (!platform) {
         platform = osinfo_platform_new(id);
         osinfo_db_add_platform(loader->priv->db, platform);
         g_object_unref(platform);
+        if (reference) {
+            g_hash_table_insert(loader->priv->entityRefs, g_strdup(id), platform);
+        }
+    } else {
+        if (!reference) {
+            g_hash_table_remove(loader->priv->entityRefs, id);
+        }
     }
     return platform;
 }
 
 static OsinfoInstallScript *osinfo_loader_get_install_script(OsinfoLoader *loader,
-                                                             const gchar *id)
+                                                             const gchar *id,
+                                                             gboolean reference)
 {
     OsinfoInstallScript *script = osinfo_db_get_install_script(loader->priv->db, id);
     if (!script) {
         script = osinfo_install_script_new(id);
         osinfo_db_add_install_script(loader->priv->db, script);
         g_object_unref(script);
+        if (reference) {
+            g_hash_table_insert(loader->priv->entityRefs, g_strdup(id), script);
+        }
+    } else {
+        if (!reference) {
+            g_hash_table_remove(loader->priv->entityRefs, id);
+        }
     }
     return script;
 }
@@ -490,7 +537,7 @@ static void osinfo_loader_device(OsinfoLoader *loader,
         return;
     }
 
-    OsinfoDevice *device = osinfo_loader_get_device(loader, id);
+    OsinfoDevice *device = osinfo_loader_get_device(loader, id, FALSE);
     xmlFree(id);
 
     osinfo_loader_entity(loader, OSINFO_ENTITY(device), keys, ctxt, root, err);
@@ -519,7 +566,7 @@ static void osinfo_loader_device_link(OsinfoLoader *loader,
             OSINFO_ERROR(err, _("Missing device link id property"));
             goto cleanup;
         }
-        OsinfoDevice *dev = osinfo_loader_get_device(loader, id);
+        OsinfoDevice *dev = osinfo_loader_get_device(loader, id, TRUE);
         xmlFree(id);
 
         OsinfoDeviceLink *devlink = NULL;
@@ -571,9 +618,9 @@ static void osinfo_loader_product_relshp(OsinfoLoader *loader,
         }
         OsinfoProduct *relproduct;
         if (OSINFO_IS_PLATFORM(product))
-            relproduct = OSINFO_PRODUCT(osinfo_loader_get_platform(loader, id));
+            relproduct = OSINFO_PRODUCT(osinfo_loader_get_platform(loader, id, TRUE));
         else
-            relproduct = OSINFO_PRODUCT(osinfo_loader_get_os(loader, id));
+            relproduct = OSINFO_PRODUCT(osinfo_loader_get_os(loader, id, TRUE));
         xmlFree(id);
 
         osinfo_product_add_related(product, relshp, relproduct);
@@ -642,7 +689,7 @@ static void osinfo_loader_platform(OsinfoLoader *loader,
         return;
     }
 
-    OsinfoPlatform *platform = osinfo_loader_get_platform(loader, id);
+    OsinfoPlatform *platform = osinfo_loader_get_platform(loader, id, FALSE);
     xmlFree(id);
 
     osinfo_loader_entity(loader, OSINFO_ENTITY(platform), NULL, ctxt, root, err);
@@ -676,7 +723,7 @@ static void osinfo_loader_deployment(OsinfoLoader *loader,
         xmlFree(id);
         return;
     }
-    OsinfoOs *os = osinfo_loader_get_os(loader, osid);
+    OsinfoOs *os = osinfo_loader_get_os(loader, osid, TRUE);
     g_free(osid);
 
     gchar *platformid = osinfo_loader_string("string(./platform/@id)", loader,
@@ -686,7 +733,7 @@ static void osinfo_loader_deployment(OsinfoLoader *loader,
         xmlFree(id);
         return;
     }
-    OsinfoPlatform *platform = osinfo_loader_get_platform(loader, platformid);
+    OsinfoPlatform *platform = osinfo_loader_get_platform(loader, platformid, TRUE);
     g_free(platformid);
 
     OsinfoDeployment *deployment = osinfo_deployment_new(id, os, platform);
@@ -724,7 +771,7 @@ static void osinfo_loader_datamap(OsinfoLoader *loader,
         return;
     }
 
-    OsinfoDatamap *map = osinfo_loader_get_datamap(loader, id);
+    OsinfoDatamap *map = osinfo_loader_get_datamap(loader, id, FALSE);
 
     nnodes = osinfo_loader_nodeset("./entry", loader, ctxt, &nodes, err);
     if (error_is_set(err))
@@ -773,7 +820,7 @@ static void osinfo_loader_install_config_params(OsinfoLoader *loader,
                                                param);
         if (mapid != NULL) {
             OsinfoDatamap *map;
-            map = osinfo_loader_get_datamap(loader, mapid);
+            map = osinfo_loader_get_datamap(loader, mapid, TRUE);
             if (map != NULL)
                 osinfo_install_config_param_set_value_map(param, map);
         }
@@ -843,7 +890,8 @@ static void osinfo_loader_install_script(OsinfoLoader *loader,
     }
 
     OsinfoInstallScript *installScript = osinfo_loader_get_install_script(loader,
-                                                                          id);
+                                                                          id,
+                                                                          FALSE);
     xmlFree(id);
 
     osinfo_loader_entity(loader, OSINFO_ENTITY(installScript), keys, ctxt, root, err);
@@ -1254,7 +1302,8 @@ static OsinfoDeviceDriver *osinfo_loader_driver(OsinfoLoader *loader,
                           OSINFO_DEVICE_DRIVER_PROP_DEVICE) == 0) {
             xmlChar *device_id = xmlGetProp(nodes[i], BAD_CAST "id");
             OsinfoDevice *device = osinfo_loader_get_device(loader,
-                                                            (gchar *)device_id);
+                                                            (gchar *)device_id,
+                                                            TRUE);
             xmlFree(device_id);
 
             osinfo_device_driver_add_device(driver, device);
@@ -1289,7 +1338,7 @@ static void osinfo_loader_os(OsinfoLoader *loader,
         return;
     }
 
-    OsinfoOs *os = osinfo_loader_get_os(loader, id);
+    OsinfoOs *os = osinfo_loader_get_os(loader, id, FALSE);
 
     osinfo_loader_entity(loader, OSINFO_ENTITY(os), keys, ctxt, root, err);
     if (error_is_set(err))
@@ -1399,7 +1448,7 @@ static void osinfo_loader_os(OsinfoLoader *loader,
             goto cleanup;
         }
         OsinfoInstallScript *script;
-        script = osinfo_loader_get_install_script(loader, scriptid);
+        script = osinfo_loader_get_install_script(loader, scriptid, TRUE);
         xmlFree(scriptid);
 
         osinfo_os_add_install_script(os, script);
@@ -1651,7 +1700,7 @@ osinfo_loader_process_file_reg_ids(OsinfoLoader *loader,
                 gchar *id = g_strdup_printf("%s/%s/%s",
                                             baseURI, vendor_id, device_id);
 
-                OsinfoDevice *dev = osinfo_loader_get_device(loader, id);
+                OsinfoDevice *dev = osinfo_loader_get_device(loader, id, FALSE);
                 OsinfoEntity *entity = OSINFO_ENTITY(dev);
                 osinfo_entity_set_param(entity,
                                         OSINFO_DEVICE_PROP_VENDOR_ID,
@@ -1908,6 +1957,13 @@ void osinfo_loader_process_default_path(OsinfoLoader *loader, GError **err)
     if (error)
         goto error;
 
+    GHashTableIter iter;
+    gpointer key, value;
+    g_hash_table_iter_init(&iter, loader->priv->entityRefs);
+    while (g_hash_table_iter_next(&iter, &key, &value)) {
+        g_warning("Entity %s referenced but not defined", (const char *)key);
+    }
+    g_hash_table_remove_all(loader->priv->entityRefs);
     return;
 
  error:
-- 
2.4.3




More information about the Libosinfo mailing list