libudev: replace unique list entries in place

udev_list_entry_add() freed the existing entry for a name before inserting
its replacement. Today that cannot lose the old entry, but only because
re-adding the key that was just removed never makes the hashmap grow, which
is an internal detail of the hashmap. Use hashmap_ensure_replace() instead:
it updates the existing bucket in place, making it explicit that replacing
an entry neither allocates nor can drop the previous value under OOM.

Follow-up for c01130824f22ec3835a35c8ab5f9aea65195a40f

diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c
index 0385c4679e97621698efe58eeb2a9f90264e2495..3ea77507c9f23b990dd44a441d4ea2293728757b 100644 (file)
--- a/src/libudev/libudev-list.c
+++ b/src/libudev/libudev-list.c
@@ -40,7 +40,7 @@ static struct udev_list_entry* udev_list_entry_free(struct udev_list_entry *entr
 
         if (entry->list) {
                 if (entry->list->unique && entry->name)
-                        hashmap_remove(entry->list->unique_entries, entry->name);
+                        hashmap_remove_value(entry->list->unique_entries, entry->name, entry);
 
                 if (!entry->list->unique || entry->list->uptodate)
                         LIST_REMOVE(entries, entry->list->entries, entry);
@@ -90,11 +90,19 @@ struct udev_list_entry* udev_list_entry_add(struct udev_list *list, const char *
                 return NULL;
 
         if (list->unique) {
-                udev_list_entry_free(hashmap_get(list->unique_entries, entry->name));
+                struct udev_list_entry *old;
 
-                if (hashmap_ensure_put(&list->unique_entries, &udev_list_entry_hash_ops, entry->name, entry) < 0)
+                /* Replace the entry for an already known name in a single step. hashmap_replace() reuses the
+                 * existing bucket and hence never allocates, so an existing entry is never dropped, not even
+                 * under memory pressure. Allocating may only fail when adding a genuinely new name, in which
+                 * case there is nothing to lose. */
+                old = hashmap_get(list->unique_entries, entry->name);
+
+                if (hashmap_ensure_replace(&list->unique_entries, &udev_list_entry_hash_ops, entry->name, entry) < 0)
                         return NULL;
 
+                udev_list_entry_free(old);
+
                 list->uptodate = false;
         } else
                 LIST_APPEND(entries, list->entries, entry);