hashmap,prioq: check hash_ops or compare_func is identical to one already set (#37379)

src/basic/hashmap.c

@@ -824,8 +824,10 @@ static int hashmap_base_ensure_allocated(HashmapBase **h, const struct hash_ops
 
         assert(h);
 
-        if (*h)
+        if (*h) {
+                assert((*h)->hash_ops == (hash_ops ?: &trivial_hash_ops));
                 return 0;
+        }
 
         q = hashmap_base_new(hash_ops, type);
         if (!q)
@@ -843,7 +845,7 @@ int ordered_hashmap_ensure_allocated(OrderedHashmap **h, const struct hash_ops *
         return hashmap_base_ensure_allocated((HashmapBase**)h, hash_ops, HASHMAP_TYPE_ORDERED);
 }
 
-int _set_ensure_allocated(Set **s, const struct hash_ops *hash_ops) {
+int set_ensure_allocated(Set **s, const struct hash_ops *hash_ops) {
         return hashmap_base_ensure_allocated((HashmapBase**)s, hash_ops, HASHMAP_TYPE_SET);
 }
 
@@ -1285,7 +1287,7 @@ int set_put(Set *s, const void *key) {
 int set_ensure_put(Set **s, const struct hash_ops *hash_ops, const void *key) {
         int r;
 
-        r = _set_ensure_allocated(s, hash_ops);
+        r = set_ensure_allocated(s, hash_ops);
         if (r < 0)
                 return r;
 
@@ -1883,7 +1885,7 @@ int set_put_strndup_full(Set **s, const struct hash_ops *hash_ops, const char *p
         assert(s);
         assert(p);
 
-        r = _set_ensure_allocated(s, hash_ops);
+        r = set_ensure_allocated(s, hash_ops);
         if (r < 0)
                 return r;
 

src/basic/prioq.c

@@ -58,8 +58,10 @@ Prioq* prioq_free(Prioq *q) {
 int prioq_ensure_allocated(Prioq **q, compare_func_t compare_func) {
         assert(q);
 
-        if (*q)
+        if (*q) {
+                assert((*q)->compare_func == compare_func);
                 return 0;
+        }
 
         *q = prioq_new(compare_func);
         if (!*q)
@@ -167,7 +169,7 @@ int prioq_put(Prioq *q, void *data, unsigned *idx) {
         return 0;
 }
 
-int prioq_ensure_put(Prioq **q, compare_func_t compare_func, void *data, unsigned *idx) {
+int _prioq_ensure_put(Prioq **q, compare_func_t compare_func, void *data, unsigned *idx) {
         int r;
 
         r = prioq_ensure_allocated(q, compare_func);

src/basic/prioq.h

@@ -16,7 +16,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Prioq*, prioq_free);
 int prioq_ensure_allocated(Prioq **q, compare_func_t compare_func);
 
 int prioq_put(Prioq *q, void *data, unsigned *idx);
-int prioq_ensure_put(Prioq **q, compare_func_t compare_func, void *data, unsigned *idx);
+int _prioq_ensure_put(Prioq **q, compare_func_t compare_func, void *data, unsigned *idx);
+#define prioq_ensure_put(q, compare_func, data, idx)                    \
+        ({                                                              \
+                int (*_func_)(const typeof((data)[0])*, const typeof((data)[0])*) = compare_func; \
+                _prioq_ensure_put(q, (compare_func_t) _func_, data, idx); \
+        })
+
 int prioq_remove(Prioq *q, void *data, unsigned *idx);
 void prioq_reshuffle(Prioq *q, void *data, unsigned *idx);
 

src/basic/set.h

@@ -16,8 +16,7 @@ static inline Set* set_free(Set *s) {
 
 #define set_copy(s) ((Set*) _hashmap_copy(HASHMAP_BASE(s)))
 
-int _set_ensure_allocated(Set **s, const struct hash_ops *hash_ops);
-#define set_ensure_allocated(h, ops) _set_ensure_allocated(h, ops)
+int set_ensure_allocated(Set **s, const struct hash_ops *hash_ops);
 
 int set_put(Set *s, const void *key);
 /* no set_update */
@@ -103,7 +102,7 @@ int set_put_strndup_full(Set **s, const struct hash_ops *hash_ops, const char *p
 #define set_put_strndup(s, p, n) set_put_strndup_full(s, &string_hash_ops_free, p, n)
 #define set_put_strdup(s, p) set_put_strndup(s, p, SIZE_MAX)
 
-int set_put_strdupv_full(Set **s, const struct hash_ops *hash_ops, char **l );
+int set_put_strdupv_full(Set **s, const struct hash_ops *hash_ops, char **l);
 #define set_put_strdupv(s, l) set_put_strdupv_full(s, &string_hash_ops_free, l)
 
 int set_put_strsplit(Set *s, const char *v, const char *separators, ExtractFlags flags);

src/core/unit.c

@@ -4629,7 +4629,7 @@ int unit_write_setting(Unit *u, UnitWriteFlags flags, const char *name, const ch
         /* Make sure the drop-in dir is registered in our path cache. This way we don't need to stupidly
          * recreate the cache after every drop-in we write. */
         if (u->manager->unit_path_cache) {
-                r = set_put_strdup(&u->manager->unit_path_cache, p);
+                r = set_put_strdup_full(&u->manager->unit_path_cache, &path_hash_ops_free, p);
                 if (r < 0)
                         return r;
         }

src/libsystemd-network/lldp-neighbor.c

@@ -37,9 +37,7 @@ DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
         sd_lldp_neighbor,
         lldp_neighbor_unlink);
 
-int lldp_neighbor_prioq_compare_func(const void *a, const void *b) {
-        const sd_lldp_neighbor *x = a, *y = b;
-
+int lldp_neighbor_prioq_compare_func(const sd_lldp_neighbor *x, const sd_lldp_neighbor *y) {
         assert(x);
         assert(y);
 

src/libsystemd-network/lldp-neighbor.h

@@ -84,7 +84,7 @@ static inline void* LLDP_NEIGHBOR_TLV_DATA(const sd_lldp_neighbor *n) {
 
 extern const struct hash_ops lldp_neighbor_hash_ops;
 int lldp_neighbor_id_compare_func(const LLDPNeighborID *x, const LLDPNeighborID *y);
-int lldp_neighbor_prioq_compare_func(const void *a, const void *b);
+int lldp_neighbor_prioq_compare_func(const sd_lldp_neighbor *x, const sd_lldp_neighbor *y);
 
 sd_lldp_neighbor *lldp_neighbor_unlink(sd_lldp_neighbor *n);
 sd_lldp_neighbor *lldp_neighbor_new(size_t raw_size);

src/resolve/resolved-link-bus.c

@@ -675,10 +675,6 @@ int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, v
         if (r < 0)
                 return r;
 
-        ns = set_new(&dns_name_hash_ops_free);
-        if (!ns)
-                return -ENOMEM;
-
         r = sd_bus_message_read_strv(message, &ntas);
         if (r < 0)
                 return r;
@@ -691,7 +687,7 @@ int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, v
                         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
                                                  "Invalid negative trust anchor domain: %s", *i);
 
-                r = set_put_strdup(&ns, *i);
+                r = set_put_strdup_full(&ns, &dns_name_hash_ops_free, *i);
                 if (r < 0)
                         return r;
 

src/resolve/resolved-link.c

@@ -477,11 +477,7 @@ static int link_update_dnssec_negative_trust_anchors(Link *l) {
         if (r < 0)
                 return r;
 
-        ns = set_new(&dns_name_hash_ops_free);
-        if (!ns)
-                return -ENOMEM;
-
-        r = set_put_strdupv(&ns, ntas);
+        r = set_put_strdupv_full(&ns, &dns_name_hash_ops_free, ntas);
         if (r < 0)
                 return r;
 

src/test/test-hashmap-plain.c

@@ -419,17 +419,10 @@ TEST(hashmap_remove_and_replace) {
 
 TEST(hashmap_ensure_allocated) {
         _cleanup_hashmap_free_ Hashmap *m = NULL;
-        int r;
 
-        r = hashmap_ensure_allocated(&m, &string_hash_ops);
-        assert_se(r == 1);
-
-        r = hashmap_ensure_allocated(&m, &string_hash_ops);
-        assert_se(r == 0);
-
-        /* different hash ops shouldn't matter at this point */
-        r = hashmap_ensure_allocated(&m, &trivial_hash_ops);
-        assert_se(r == 0);
+        ASSERT_OK_POSITIVE(hashmap_ensure_allocated(&m, &string_hash_ops));
+        ASSERT_OK_ZERO(hashmap_ensure_allocated(&m, &string_hash_ops));
+        ASSERT_SIGNAL(hashmap_ensure_allocated(&m, &trivial_hash_ops), SIGABRT);
 }
 
 TEST(hashmap_foreach_key) {

src/test/test-set.c

@@ -123,10 +123,10 @@ TEST(set_put_strdupv) {
 TEST(set_ensure_allocated) {
         _cleanup_set_free_ Set *m = NULL;
 
-        assert_se(set_ensure_allocated(&m, &string_hash_ops) == 1);
-        assert_se(set_ensure_allocated(&m, &string_hash_ops) == 0);
-        assert_se(set_ensure_allocated(&m, NULL) == 0);
-        assert_se(set_isempty(m));
+        ASSERT_OK_POSITIVE(set_ensure_allocated(&m, &string_hash_ops));
+        ASSERT_OK_ZERO(set_ensure_allocated(&m, &string_hash_ops));
+        ASSERT_SIGNAL(set_ensure_allocated(&m, NULL), SIGABRT);
+        ASSERT_TRUE(set_isempty(m));
 }
 
 TEST(set_copy) {
@@ -159,13 +159,13 @@ TEST(set_copy) {
 TEST(set_ensure_put) {
         _cleanup_set_free_ Set *m = NULL;
 
-        assert_se(set_ensure_put(&m, &string_hash_ops, "a") == 1);
-        assert_se(set_ensure_put(&m, &string_hash_ops, "a") == 0);
-        assert_se(set_ensure_put(&m, NULL, "a") == 0);
-        assert_se(set_ensure_put(&m, &string_hash_ops, "b") == 1);
-        assert_se(set_ensure_put(&m, &string_hash_ops, "b") == 0);
-        assert_se(set_ensure_put(&m, &string_hash_ops, "a") == 0);
-        assert_se(set_size(m) == 2);
+        ASSERT_OK_POSITIVE(set_ensure_put(&m, &string_hash_ops, "a"));
+        ASSERT_OK_ZERO(set_ensure_put(&m, &string_hash_ops, "a"));
+        ASSERT_SIGNAL(set_ensure_put(&m, NULL, "a"), SIGABRT);
+        ASSERT_OK_POSITIVE(set_ensure_put(&m, &string_hash_ops, "b"));
+        ASSERT_OK_ZERO(set_ensure_put(&m, &string_hash_ops, "b"));
+        ASSERT_OK_ZERO(set_ensure_put(&m, &string_hash_ops, "a"));
+        ASSERT_EQ(set_size(m), 2u);
 }
 
 TEST(set_ensure_consume) {