]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
test-ordered-set: add a case where we get 0 for duplicate entries 16561/head
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Thu, 23 Jul 2020 13:47:21 +0000 (15:47 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Thu, 23 Jul 2020 13:47:21 +0000 (15:47 +0200)
This API is a complete mess. We forgot to do a hashed comparison for duplicate
entries and we use a direct pointer comparison. For trivial_hash_ops the result
is the same. For all other case, it's not. Fixing this properly will require
auditing all the uses of set_put() and ordered_set_put(). For now, let's just
acknowledge the breakage.

src/test/test-ordered-set.c

index 581b0aa6a1a4bebaf60b2764947e43d28b4e258b..268c54fccc78ab54cc81cc52e5331135c63b3390 100644 (file)
@@ -7,6 +7,8 @@
 #include "strv.h"
 
 static void test_set_steal_first(void) {
+        log_info("/* %s */", __func__);
+
         _cleanup_ordered_set_free_ OrderedSet *m = NULL;
         int seen[3] = {};
         char *val;
@@ -42,12 +44,18 @@ DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_hash_ops, void, trivial_hash_
 static void test_set_free_with_hash_ops(void) {
         OrderedSet *m;
         struct Item items[4] = {};
-        unsigned i;
+
+        log_info("/* %s */", __func__);
 
         assert_se(m = ordered_set_new(&item_hash_ops));
-        for (i = 0; i < ELEMENTSOF(items) - 1; i++)
+
+        for (size_t i = 0; i < ELEMENTSOF(items) - 1; i++)
                 assert_se(ordered_set_put(m, items + i) == 1);
 
+        for (size_t i = 0; i < ELEMENTSOF(items) - 1; i++)
+                assert_se(ordered_set_put(m, items + i) == 0);  /* We get 0 here, because we use trivial hash
+                                                                 * ops. Also see below... */
+
         m = ordered_set_free(m);
         assert_se(items[0].seen == 1);
         assert_se(items[1].seen == 1);
@@ -59,6 +67,8 @@ static void test_set_put(void) {
         _cleanup_ordered_set_free_ OrderedSet *m = NULL;
         _cleanup_free_ char **t = NULL, *str = NULL;
 
+        log_info("/* %s */", __func__);
+
         m = ordered_set_new(&string_hash_ops);
         assert_se(m);
 
@@ -72,7 +82,8 @@ static void test_set_put(void) {
         assert_se(ordered_set_put(m, (void*) "22") == 0);
 
         assert_se(str = strdup("333"));
-        assert_se(ordered_set_put(m, str) == -EEXIST);
+        assert_se(ordered_set_put(m, str) == -EEXIST); /* ... and we get -EEXIST here, because we use
+                                                        * non-trivial hash ops. */
 
         assert_se(t = ordered_set_get_strv(m));
         assert_se(streq(t[0], "1"));
@@ -89,6 +100,8 @@ static void test_set_put_string_set(void) {
         _cleanup_free_ char **final = NULL; /* "just free" because the strings are in the set */
         void *t;
 
+        log_info("/* %s */", __func__);
+
         m = ordered_set_new(&string_hash_ops);
         assert_se(m);