From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Wed, 29 Apr 2020 07:55:28 +0000 (+0200)
Subject: basic/hashmap: allow NULL values in strdup hashmaps and add test
X-Git-Tag: v246-rc1~418^2~5
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=25b3e2a8355e2f92767d042e85d7dbb1d54ad6d4;p=thirdparty%2Fsystemd.git

basic/hashmap: allow NULL values in strdup hashmaps and add test
---

diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c
index 7abe62fa937..efbe95bb9e3 100644
--- a/src/basic/hashmap.c
+++ b/src/basic/hashmap.c
@@ -1775,22 +1775,30 @@ int hashmap_put_strdup(Hashmap **h, const char *k, const char *v) {
                 return r;
 
         _cleanup_free_ char *kdup = NULL, *vdup = NULL;
+
         kdup = strdup(k);
-        vdup = strdup(v);
-        if (!kdup || !vdup)
+        if (!kdup)
                 return -ENOMEM;
 
+        if (v) {
+                vdup = strdup(v);
+                if (!vdup)
+                        return -ENOMEM;
+        }
+
         r = hashmap_put(*h, kdup, vdup);
         if (r < 0) {
-                if (r == -EEXIST && streq(v, hashmap_get(*h, kdup)))
+                if (r == -EEXIST && streq_ptr(v, hashmap_get(*h, kdup)))
                         return 0;
                 return r;
         }
 
-        assert(r > 0); /* 0 would mean vdup is already in the hashmap, which cannot be */
-        kdup = vdup = NULL;
+        /* 0 with non-null vdup would mean vdup is already in the hashmap, which cannot be */
+        assert(vdup == NULL || r > 0);
+        if (r > 0)
+                kdup = vdup = NULL;
 
-        return 0;
+        return r;
 }
 
 int set_put_strdup(Set **s, const char *p) {
diff --git a/src/test/test-hashmap.c b/src/test/test-hashmap.c
index 1a6e8ffa58e..94dbbf15769 100644
--- a/src/test/test-hashmap.c
+++ b/src/test/test-hashmap.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
 #include "hashmap.h"
+#include "string-util.h"
 #include "util.h"
 
 unsigned custom_counter = 0;
@@ -109,6 +110,58 @@ static void test_iterated_cache(void) {
         assert_se(iterated_cache_free(c) == NULL);
 }
 
+static void test_hashmap_put_strdup(void) {
+        _cleanup_hashmap_free_ Hashmap *m = NULL;
+        char *s;
+
+        /* We don't have ordered_hashmap_put_strdup() yet. If it is added,
+         * these tests should be moved to test-hashmap-plain.c. */
+
+        log_info("/* %s */", __func__);
+
+        assert_se(hashmap_put_strdup(&m, "foo", "bar") == 1);
+        assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0);
+        assert_se(hashmap_put_strdup(&m, "foo", "BAR") == -EEXIST);
+        assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0);
+        assert_se(hashmap_contains(m, "foo"));
+
+        s = hashmap_get(m, "foo");
+        assert_se(streq(s, "bar"));
+
+        assert_se(hashmap_put_strdup(&m, "xxx", "bar") == 1);
+        assert_se(hashmap_put_strdup(&m, "xxx", "bar") == 0);
+        assert_se(hashmap_put_strdup(&m, "xxx", "BAR") == -EEXIST);
+        assert_se(hashmap_put_strdup(&m, "xxx", "bar") == 0);
+        assert_se(hashmap_contains(m, "xxx"));
+
+        s = hashmap_get(m, "xxx");
+        assert_se(streq(s, "bar"));
+}
+
+static void test_hashmap_put_strdup_null(void) {
+        _cleanup_hashmap_free_ Hashmap *m = NULL;
+        char *s;
+
+        log_info("/* %s */", __func__);
+
+        assert_se(hashmap_put_strdup(&m, "foo", "bar") == 1);
+        assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0);
+        assert_se(hashmap_put_strdup(&m, "foo", NULL) == -EEXIST);
+        assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0);
+        assert_se(hashmap_contains(m, "foo"));
+
+        s = hashmap_get(m, "foo");
+        assert_se(streq(s, "bar"));
+
+        assert_se(hashmap_put_strdup(&m, "xxx", NULL) == 1);
+        assert_se(hashmap_put_strdup(&m, "xxx", "bar") == -EEXIST);
+        assert_se(hashmap_put_strdup(&m, "xxx", NULL) == 0);
+        assert_se(hashmap_contains(m, "xxx"));
+
+        s = hashmap_get(m, "xxx");
+        assert_se(s == NULL);
+}
+
 int main(int argc, const char *argv[]) {
         /* This file tests in test-hashmap-plain.c, and tests in test-hashmap-ordered.c, which is generated
          * from test-hashmap-plain.c. Hashmap tests should be added to test-hashmap-plain.c, and here only if
@@ -127,6 +180,8 @@ int main(int argc, const char *argv[]) {
         test_trivial_compare_func();
         test_string_compare_func();
         test_iterated_cache();
+        test_hashmap_put_strdup();
+        test_hashmap_put_strdup_null();
 
         return 0;
 }