From 7ab62437e34ee9b9ea99e82355f82c8e54410d18 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Mon, 16 Jun 2025 18:33:32 +0100 Subject: [PATCH 1/3] testsuite/test-hash: introduce and use OK() macro The current assert_return() macro has a few short comings: - the name implies assert() - all users unnecessarily provide EXIT_FAILURE - the users cannot provide a message - "Failed assertion:" isn't great Tweak it up and in the process bump it to ALL_CAPS, to better indicate that it's a macro and not a function. Preserve the original, since we're converting one test at a time - starting with test-hash. Signed-off-by: Emil Velikov --- testsuite/test-hash.c | 48 +++++++++++++++++++++---------------------- testsuite/testsuite.h | 16 ++++++++------- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/testsuite/test-hash.c b/testsuite/test-hash.c index 810e7d2c..2209b1a1 100644 --- a/testsuite/test-hash.c +++ b/testsuite/test-hash.c @@ -25,7 +25,7 @@ static void countfreecalls(_maybe_unused_ void *v) static int test_hash_new(void) { struct hash *h = hash_new(8, NULL); - assert_return(h != NULL, EXIT_FAILURE); + OK(h != NULL, "Cannot create hash table"); hash_free(h); return 0; } @@ -41,7 +41,7 @@ static int test_hash_get_count(void) hash_add(h, k2, v2); hash_add(h, k3, v3); - assert_return(hash_get_count(h) == 3, EXIT_FAILURE); + OK(hash_get_count(h) == 3, "Wrong number of hash entries"); hash_free(h); return 0; @@ -63,13 +63,13 @@ static int test_hash_replace(void) /* replace v1 */ r |= hash_add(h, k1, v4); - assert_return(r == 0, EXIT_FAILURE); - assert_return(hash_get_count(h) == 3, EXIT_FAILURE); + OK(r == 0, "Failed to add hash entry(ies)"); + OK(hash_get_count(h) == 3, "Wrong number of hash entries"); v = hash_find(h, "k1"); - assert_return(streq(v, v4), EXIT_FAILURE); + OK(streq(v, v4), "Found incorrect hash entry"); - assert_return(freecount == 1, EXIT_FAILURE); + OK(freecount == 1, "Incorrect free hash entries"); hash_free(h); return 0; @@ -88,17 +88,17 @@ static int test_hash_replace_failing(void) r |= hash_add(h, k2, v2); r |= hash_add(h, k3, v3); - assert_return(r == 0, EXIT_FAILURE); + OK(r == 0, "Failed to add hash entry(ies)"); /* replace v1 */ r = hash_add_unique(h, k1, v4); - assert_return(r != 0, EXIT_FAILURE); - assert_return(hash_get_count(h) == 3, EXIT_FAILURE); + OK(r != 0, "Failed to add unique hash entry"); + OK(hash_get_count(h) == 3, "Wrong number of hash entries"); v = hash_find(h, "k1"); - assert_return(streq(v, v1), EXIT_FAILURE); + OK(streq(v, v1), "Found incorrect hash entry"); - assert_return(freecount == 0, EXIT_FAILURE); + OK(freecount == 0, "Incorrect free hash entries"); hash_free(h); return 0; @@ -124,12 +124,12 @@ static int test_hash_iter(void) for (hash_iter_init(h, &iter); hash_iter_next(&iter, &k, (const void **)&v);) { v2 = hash_find(h2, k); - assert_return(v2 != NULL, EXIT_FAILURE); + OK(v2 != NULL, "Cannot find hash entry"); hash_del(h2, k); } - assert_return(hash_get_count(h) == 3, EXIT_FAILURE); - assert_return(hash_get_count(h2) == 0, EXIT_FAILURE); + OK(hash_get_count(h) == 3, "Wrong number of hash entries"); + OK(hash_get_count(h2) == 0, "Wrong number of hash entries"); hash_free(h); hash_free(h2); @@ -157,12 +157,12 @@ static int test_hash_iter_after_del(void) for (hash_iter_init(h, &iter); hash_iter_next(&iter, &k, (const void **)&v);) { v2 = hash_find(h2, k); - assert_return(v2 != NULL, EXIT_FAILURE); + OK(v2 != NULL, "Cannot find hash entry"); hash_del(h2, k); } - assert_return(hash_get_count(h) == 2, EXIT_FAILURE); - assert_return(hash_get_count(h2) == 1, EXIT_FAILURE); + OK(hash_get_count(h) == 2, "Wrong number of hash entries"); + OK(hash_get_count(h2) == 1, "Wrong number of hash entries"); hash_free(h); hash_free(h2); @@ -193,7 +193,7 @@ static int test_hash_del_nonexistent(void) int rc; rc = hash_del(h, k1); - assert_return(rc == -ENOENT, EXIT_FAILURE); + OK(rc == -ENOENT, "Incorrect ret code deleting non-existent entry"); hash_free(h); @@ -214,13 +214,13 @@ static int test_hash_free(void) hash_del(h, k1); - assert_return(freecount == 1, EXIT_FAILURE); + OK(freecount == 1, "Incorrect free hash entries"); - assert_return(hash_get_count(h) == 2, EXIT_FAILURE); + OK(hash_get_count(h) == 2, "Wrong number of hash entries"); hash_free(h); - assert_return(freecount == 3, EXIT_FAILURE); + OK(freecount == 3, "Incorrect free hash entries"); return 0; } @@ -244,7 +244,7 @@ static int test_hash_add_unique(void) hash_add_unique(h, k[idx], v[idx]); } - assert_return(hash_get_count(h) == N, EXIT_FAILURE); + OK(hash_get_count(h) == N, "Wrong number of hash entries"); hash_free(h); } return 0; @@ -268,7 +268,7 @@ static int test_hash_massive_add_del(void) k += 8; } - assert_return(hash_get_count(h) == N, EXIT_FAILURE); + OK(hash_get_count(h) == N, "Wrong number of hash entries"); k = &buf[0]; for (i = 0; i < N; i++) { @@ -276,7 +276,7 @@ static int test_hash_massive_add_del(void) k += 8; } - assert_return(hash_get_count(h) == 0, EXIT_FAILURE); + OK(hash_get_count(h) == 0, "Wrong number of hash entries"); hash_free(h); return 0; diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h index ae17a321..f79bebbf 100644 --- a/testsuite/testsuite.h +++ b/testsuite/testsuite.h @@ -119,13 +119,15 @@ int test_run(const struct test *t); #define WARN(fmt, ...) _LOG("WARN: ", fmt, ##__VA_ARGS__) #define ERR(fmt, ...) _LOG("ERR: ", fmt, ##__VA_ARGS__) -#define assert_return(expr, r) \ - do { \ - if ((!(expr))) { \ - ERR("Failed assertion: " #expr " %s:%d %s\n", __FILE__, \ - __LINE__, __PRETTY_FUNCTION__); \ - return (r); \ - } \ +#define assert_return(expr, r) OK(expr, "Failed assertion") + +#define OK(expr, msg) \ + do { \ + if ((!(expr))) { \ + ERR(msg ": " #expr " %s:%d %s\n", __FILE__, __LINE__, \ + __PRETTY_FUNCTION__); \ + return EXIT_FAILURE; \ + } \ } while (false) /* Test definitions */ From 3046502fb19ca9e38da5b22c03b1d1a4cd8ee79b Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Mon, 16 Jun 2025 18:49:18 +0100 Subject: [PATCH 2/3] testsuite/test-array: assert_return -> OK conversion Migrate to the new macro. Signed-off-by: Emil Velikov --- testsuite/test-array.c | 70 +++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/testsuite/test-array.c b/testsuite/test-array.c index 6e1f6896..fc6fee56 100644 --- a/testsuite/test-array.c +++ b/testsuite/test-array.c @@ -20,8 +20,8 @@ static int test_array_append1(void) array_init(&array, 2); array_append(&array, c1); - assert_return(array.count == 1, EXIT_FAILURE); - assert_return(array.array[0] == c1, EXIT_FAILURE); + OK(array.count == 1, "Incorrect number of array entries"); + OK(array.array[0] == c1, "Incorrect array entry"); array_free_array(&array); return 0; @@ -39,10 +39,10 @@ static int test_array_append2(void) array_append(&array, c1); array_append(&array, c2); array_append(&array, c3); - assert_return(array.count == 3, EXIT_FAILURE); - assert_return(array.array[0] == c1, EXIT_FAILURE); - assert_return(array.array[1] == c2, EXIT_FAILURE); - assert_return(array.array[2] == c3, EXIT_FAILURE); + OK(array.count == 3, "Incorrect number of array entries"); + OK(array.array[0] == c1, "Incorrect array entry"); + OK(array.array[1] == c2, "Incorrect array entry"); + OK(array.array[2] == c3, "Incorrect array entry"); array_free_array(&array); return 0; @@ -63,10 +63,10 @@ static int test_array_append_unique(void) array_append_unique(&array, c3); array_append_unique(&array, c2); array_append_unique(&array, c1); - assert_return(array.count == 3, EXIT_FAILURE); - assert_return(array.array[0] == c1, EXIT_FAILURE); - assert_return(array.array[1] == c2, EXIT_FAILURE); - assert_return(array.array[2] == c3, EXIT_FAILURE); + OK(array.count == 3, "Incorrect number of array entries"); + OK(array.array[0] == c1, "Incorrect array entry"); + OK(array.array[1] == c2, "Incorrect array entry"); + OK(array.array[2] == c3, "Incorrect array entry"); array_free_array(&array); return 0; @@ -96,13 +96,13 @@ static int test_array_sort(void) array_append(&array, c3); array_append(&array, c1); array_sort(&array, strptrcmp); - assert_return(array.count == 6, EXIT_FAILURE); - assert_return(array.array[0] == c1, EXIT_FAILURE); - assert_return(array.array[1] == c1, EXIT_FAILURE); - assert_return(array.array[2] == c2, EXIT_FAILURE); - assert_return(array.array[3] == c2, EXIT_FAILURE); - assert_return(array.array[4] == c3, EXIT_FAILURE); - assert_return(array.array[5] == c3, EXIT_FAILURE); + OK(array.count == 6, "Incorrect number of array entries"); + OK(array.array[0] == c1, "Incorrect array entry"); + OK(array.array[1] == c1, "Incorrect array entry"); + OK(array.array[2] == c2, "Incorrect array entry"); + OK(array.array[3] == c2, "Incorrect array entry"); + OK(array.array[4] == c3, "Incorrect array entry"); + OK(array.array[5] == c3, "Incorrect array entry"); array_free_array(&array); return 0; @@ -122,29 +122,29 @@ static int test_array_remove_at(void) array_append(&array, c3); array_remove_at(&array, 2); - assert_return(array.count == 2, EXIT_FAILURE); - assert_return(array.array[0] == c1, EXIT_FAILURE); - assert_return(array.array[1] == c2, EXIT_FAILURE); - assert_return(array.total == 4, EXIT_FAILURE); + OK(array.count == 2, "Incorrect number of array entries"); + OK(array.array[0] == c1, "Incorrect array entry"); + OK(array.array[1] == c2, "Incorrect array entry"); + OK(array.total == 4, "Incorrect array size"); array_remove_at(&array, 0); - assert_return(array.count == 1, EXIT_FAILURE); - assert_return(array.array[0] == c2, EXIT_FAILURE); - assert_return(array.total == 2, EXIT_FAILURE); + OK(array.count == 1, "Incorrect number of array entries"); + OK(array.array[0] == c2, "Incorrect array entry"); + OK(array.total == 2, "Incorrect array size"); array_remove_at(&array, 0); - assert_return(array.count == 0, EXIT_FAILURE); - assert_return(array.total == 2, EXIT_FAILURE); + OK(array.count == 0, "Incorrect number of array entries"); + OK(array.total == 2, "Incorrect array size"); array_append(&array, c1); array_append(&array, c2); array_append(&array, c3); array_remove_at(&array, 1); - assert_return(array.count == 2, EXIT_FAILURE); - assert_return(array.array[0] == c1, EXIT_FAILURE); - assert_return(array.array[1] == c3, EXIT_FAILURE); - assert_return(array.total == 4, EXIT_FAILURE); + OK(array.count == 2, "Incorrect number of array entries"); + OK(array.array[0] == c1, "Incorrect array entry"); + OK(array.array[1] == c3, "Incorrect array entry"); + OK(array.total == 4, "Incorrect array size"); array_free_array(&array); @@ -166,18 +166,18 @@ static int test_array_pop(void) array_pop(&array); - assert_return(array.count == 2, EXIT_FAILURE); - assert_return(array.array[0] == c1, EXIT_FAILURE); - assert_return(array.array[1] == c2, EXIT_FAILURE); + OK(array.count == 2, "Incorrect number of array entries"); + OK(array.array[0] == c1, "Incorrect array entry"); + OK(array.array[1] == c2, "Incorrect array entry"); array_pop(&array); array_pop(&array); - assert_return(array.count == 0, EXIT_FAILURE); + OK(array.count == 0, "Incorrect number of array entries"); array_pop(&array); - assert_return(array.count == 0, EXIT_FAILURE); + OK(array.count == 0, "Incorrect number of array entries"); array_free_array(&array); From 28c5a8f64c2cb278eb0814d60e0038002a382096 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Mon, 16 Jun 2025 19:09:52 +0100 Subject: [PATCH 3/3] testsuite/test-blacklist: convert to OK() Swap the manual checks for the new OK() macro. In the process, we remove the cleanup in the error path. Many other tests don't bother, plus in the places that do try they don't always get it right/fully. Signed-off-by: Emil Velikov --- testsuite/test-blacklist.c | 29 +++++++---------------------- testsuite/testsuite.h | 14 +++++++------- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/testsuite/test-blacklist.c b/testsuite/test-blacklist.c index 83e4e266..4818c5be 100644 --- a/testsuite/test-blacklist.c +++ b/testsuite/test-blacklist.c @@ -33,50 +33,35 @@ static int blacklist_1(void) static const char *const names[] = { "pcspkr", "pcspkr2", "floppy", "ext4" }; ctx = kmod_new(NULL, NULL); - if (ctx == NULL) - return EXIT_FAILURE; + OK(ctx != NULL, "Failed to create kmod context"); for (size_t i = 0; i < ARRAY_SIZE(names); i++) { err = kmod_module_new_from_name(ctx, names[i], &mod); - if (err < 0) - goto fail_lookup; + OK(err == 0, "Failed to lookup new module"); list = kmod_list_append(list, mod); } err = kmod_module_apply_filter(ctx, KMOD_FILTER_BLACKLIST, list, &filtered); - if (err < 0) { - ERR("Could not filter: %s\n", strerror(-err)); - goto fail; - } - if (filtered == NULL) { - ERR("All modules were filtered out!\n"); - goto fail; - } + OK(err == 0, "Could not filter: %s", strerror(-err)); + OK(filtered != NULL, "All modules were filtered out!"); kmod_list_foreach(l, filtered) { const char *modname; mod = kmod_module_get_module(l); modname = kmod_module_get_name(mod); - if (streq("pcspkr", modname) || streq("floppy", modname)) - goto fail; + OK(!streq("pcspkr", modname) && !streq("floppy", modname), + "Module should have been filtered out"); len++; kmod_module_unref(mod); } - if (len != 2) - goto fail; + OK(len == 2, "Invalid number of unfiltered modules"); kmod_module_unref_list(filtered); kmod_module_unref_list(list); kmod_unref(ctx); return EXIT_SUCCESS; - -fail: - kmod_module_unref_list(list); -fail_lookup: - kmod_unref(ctx); - return EXIT_FAILURE; } DEFINE_TEST(blacklist_1, .description = "check if modules are correctly blacklisted", diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h index f79bebbf..3a08fa6f 100644 --- a/testsuite/testsuite.h +++ b/testsuite/testsuite.h @@ -121,13 +121,13 @@ int test_run(const struct test *t); #define assert_return(expr, r) OK(expr, "Failed assertion") -#define OK(expr, msg) \ - do { \ - if ((!(expr))) { \ - ERR(msg ": " #expr " %s:%d %s\n", __FILE__, __LINE__, \ - __PRETTY_FUNCTION__); \ - return EXIT_FAILURE; \ - } \ +#define OK(expr, fmt, ...) \ + do { \ + if ((!(expr))) { \ + ERR(fmt ": " #expr " %s:%d %s\n", ##__VA_ARGS__, __FILE__, \ + __LINE__, __PRETTY_FUNCTION__); \ + return EXIT_FAILURE; \ + } \ } while (false) /* Test definitions */