From 2645d4bcc16881e72817a0f7ab01cc859634ad33 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 23 Jul 2020 21:28:53 +0900 Subject: [PATCH 1/2] test: clarify that ordered_set_put() returns -EEXIST if entry is duplicated --- src/test/test-ordered-set.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/test-ordered-set.c b/src/test/test-ordered-set.c index 0d29fcfad2..581b0aa6a1 100644 --- a/src/test/test-ordered-set.c +++ b/src/test/test-ordered-set.c @@ -57,7 +57,7 @@ static void test_set_free_with_hash_ops(void) { static void test_set_put(void) { _cleanup_ordered_set_free_ OrderedSet *m = NULL; - _cleanup_free_ char **t = NULL; + _cleanup_free_ char **t = NULL, *str = NULL; m = ordered_set_new(&string_hash_ops); assert_se(m); @@ -71,6 +71,9 @@ static void test_set_put(void) { assert_se(ordered_set_put(m, (void*) "333") == 0); assert_se(ordered_set_put(m, (void*) "22") == 0); + assert_se(str = strdup("333")); + assert_se(ordered_set_put(m, str) == -EEXIST); + assert_se(t = ordered_set_get_strv(m)); assert_se(streq(t[0], "1")); assert_se(streq(t[1], "22")); From d4fa0493a75d6a94dcdd37c6124f4374e2d02cad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Jul 2020 15:47:21 +0200 Subject: [PATCH 2/2] test-ordered-set: add a case where we get 0 for duplicate entries 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 | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/test/test-ordered-set.c b/src/test/test-ordered-set.c index 581b0aa6a1..268c54fccc 100644 --- a/src/test/test-ordered-set.c +++ b/src/test/test-ordered-set.c @@ -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);