hashmap: honor the value destructor in set_ensure_consume()

Sets store their element in the key slot but use a value destructor
(DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR). On the reject/duplicate path
set_ensure_consume() only checked free_key and otherwise called free().

Follow-up for fcc1d0315d335ba31d85d6023c79d6404c62e167

diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c
index a2868e24557bdd2c9fae7f9ce131d5ccfe80d69f..89517b99f724167eeb42f77e7647a8d9c851797f 100644 (file)
--- a/src/basic/hashmap.c
+++ b/src/basic/hashmap.c
@@ -1330,6 +1330,9 @@ int set_ensure_consume(Set **s, const struct hash_ops *hash_ops, void *key) {
         if (r <= 0) {
                 if (hash_ops && hash_ops->free_key)
                         hash_ops->free_key(key);
+                else if (hash_ops && hash_ops->free_value)
+                        /* Sets store their element in the key slot but may carry a value destructor. */
+                        hash_ops->free_value(key);
                 else
                         free(key);
         }

diff --git a/src/test/test-set.c b/src/test/test-set.c
index 4c1872d636d3e35465aeca3892acd2d55eb77793..652b6837beb73faf0fb599384215b632d2723481 100644 (file)
--- a/src/test/test-set.c
+++ b/src/test/test-set.c
@@ -2,8 +2,10 @@
 
 #include <unistd.h>
 
+#include "alloc-util.h"
 #include "random-util.h"
 #include "set.h"
+#include "siphash24.h"
 #include "strv.h"
 #include "tests.h"
 
@@ -51,6 +53,46 @@ TEST(set_free_with_hash_ops) {
         assert_se(items[3].seen == 0);
 }
 
+static unsigned set_ensure_consume_freed = 0;
+
+typedef struct ValItem {
+        int key;
+} ValItem;
+
+static void val_item_free(ValItem *i) {
+        set_ensure_consume_freed++;
+        free(i);
+}
+
+static void val_item_hash_func(const ValItem *i, struct siphash *state) {
+        siphash24_compress_typesafe(i->key, state);
+}
+
+static int val_item_compare_func(const ValItem *a, const ValItem *b) {
+        return CMP(a->key, b->key);
+}
+
+DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
+                val_item_hash_ops, ValItem, val_item_hash_func, val_item_compare_func,
+                ValItem, val_item_free);
+
+TEST(set_ensure_consume_value_destructor) {
+        _cleanup_set_free_ Set *s = NULL;
+        ValItem *a, *b;
+
+        ASSERT_NOT_NULL(a = new0(ValItem, 1));
+        a->key = 1;
+        ASSERT_OK_POSITIVE(set_ensure_consume(&s, &val_item_hash_ops, a));
+
+        ASSERT_NOT_NULL(b = new0(ValItem, 1));
+        b->key = 1; /* same key, different pointer -> duplicate */
+
+        set_ensure_consume_freed = 0;
+        /* The rejected duplicate must be released via the value destructor, not plain free(). */
+        ASSERT_OK_ZERO(set_ensure_consume(&s, &val_item_hash_ops, b));
+        ASSERT_EQ(set_ensure_consume_freed, 1u);
+}
+
 TEST(set_put) {
         _cleanup_set_free_ Set *m = NULL;