[Libosinfo] [PATCH 6/8] list: Fix replacement in osinfo_list_add

Christophe Fergeau cfergeau at redhat.com
Thu Nov 5 16:20:44 UTC 2015


OsinfoList maintains a mapping between a const char *entity_id and an
OsinfoEntity object. It does so by using both a GPtrArray and a
GHashTable.

When an entity is added to an OsinfoList, a pointer from the
OsinfoEntity is used as the key (through osinfo_entity_get_id()) and
the OsinfoEntity is g_object_ref'ed and used as a value.
The OsinfoEntity is also appended to the array mentioned previously.
The use of g_hash_table_new_full() ensures that the OsinfoEntity will be
unref'ed when it's removed from the hash table.

However, when trying to add an entity with the same id as one already
present in the OsinfoList, several issues occur:
- g_hash_table_insert() is used, which means the preexisting entity with
  the same id is going to get removed, and thus g_object_unref'ed.
  However, the _old_ key will still be used by the hash table, which
  means that if the OsinfoList was holding the last reference to the
  old OsinfoEntity, the key stored in the hash table is going to
  point to freed memory
- similarly, we make no attempt at removing the preexisting entity from
  the GPtrArray, so this will also contain stale data

This commit makes sure we remove entities with the same id from the
GPtrArray when this situation occurs. It uses g_hash_table_replace()
rather than g_hash_table_insert() which replaces both the key and the
value if the hash table already contains the same key.

This fixes some valgrind issues or crashes that occurred when running
make check in a libosinfo tree with 2 databases, one in the old layout
and one using the new layout.
---
 osinfo/osinfo_list.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/osinfo/osinfo_list.c b/osinfo/osinfo_list.c
index 6e32387..c829a62 100644
--- a/osinfo/osinfo_list.c
+++ b/osinfo/osinfo_list.c
@@ -237,12 +237,17 @@ OsinfoEntity *osinfo_list_find_by_id(OsinfoList *list, const gchar *id)
  */
 void osinfo_list_add(OsinfoList *list, OsinfoEntity *entity)
 {
+    OsinfoEntity *preexisting;
     g_return_if_fail(G_TYPE_CHECK_INSTANCE_TYPE(entity, list->priv->elementType));
 
     g_object_ref(entity);
+    preexisting = osinfo_list_find_by_id(list, osinfo_entity_get_id(entity));
+    if (preexisting != NULL) {
+        g_ptr_array_remove(list->priv->array, preexisting);
+    }
     g_ptr_array_add(list->priv->array, entity);
-    g_hash_table_insert(list->priv->entities,
-                        (gchar *)osinfo_entity_get_id(entity), entity);
+    g_hash_table_replace(list->priv->entities,
+                         (gchar *)osinfo_entity_get_id(entity), entity);
 }
 
 
-- 
2.5.0




More information about the Libosinfo mailing list