From bbddd2b8d9b06bfef8b3eb58f53144fa64995083 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 Oct 2018 21:56:46 +0900 Subject: [PATCH 1/7] test: use CMP() macro at one more place --- src/test/test-prioq.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/test/test-prioq.c b/src/test/test-prioq.c index f7fb5ce422..57147a166f 100644 --- a/src/test/test-prioq.c +++ b/src/test/test-prioq.c @@ -54,13 +54,7 @@ struct test { static int test_compare(const void *a, const void *b) { const struct test *x = a, *y = b; - if (x->value < y->value) - return -1; - - if (x->value > y->value) - return 1; - - return 0; + return CMP(x->value, y->value); } static void test_hash(const void *a, struct siphash *state) { From d13b5f5a85f6070586e59f6dd2a8b2c9358d5c7e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 Oct 2018 22:17:04 +0900 Subject: [PATCH 2/7] util,test: introduce cleanup function prioq_freep() This also simplifies test-prioq.c. --- src/basic/prioq.h | 1 + src/test/test-prioq.c | 61 ++++++++++++------------------------------- 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/src/basic/prioq.h b/src/basic/prioq.h index e036175260..bba5c7caa4 100644 --- a/src/basic/prioq.h +++ b/src/basic/prioq.h @@ -12,6 +12,7 @@ typedef struct Prioq Prioq; Prioq *prioq_new(compare_func_t compare); Prioq *prioq_free(Prioq *q); +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); diff --git a/src/test/test-prioq.c b/src/test/test-prioq.c index 57147a166f..8aff66978e 100644 --- a/src/test/test-prioq.c +++ b/src/test/test-prioq.c @@ -15,17 +15,14 @@ static int unsigned_compare(const unsigned *a, const unsigned *b) { } static void test_unsigned(void) { - unsigned buffer[SET_SIZE], i; - Prioq *q; + _cleanup_(prioq_freep) Prioq *q = NULL; + unsigned buffer[SET_SIZE], i, u; srand(0); - q = prioq_new(trivial_compare_func); - assert_se(q); + assert_se(q = prioq_new(trivial_compare_func)); for (i = 0; i < ELEMENTSOF(buffer); i++) { - unsigned u; - u = (unsigned) rand(); buffer[i] = u; assert_se(prioq_put(q, UINT_TO_PTR(u), NULL) >= 0); @@ -34,8 +31,6 @@ static void test_unsigned(void) { typesafe_qsort(buffer, ELEMENTSOF(buffer), unsigned_compare); for (i = 0; i < ELEMENTSOF(buffer); i++) { - unsigned u; - assert_se(prioq_size(q) == ELEMENTSOF(buffer) - i); u = PTR_TO_UINT(prioq_pop(q)); @@ -43,7 +38,6 @@ static void test_unsigned(void) { } assert_se(prioq_isempty(q)); - prioq_free(q); } struct test { @@ -69,66 +63,45 @@ static const struct hash_ops test_hash_ops = { }; static void test_struct(void) { - Prioq *q; - Set *s; + _cleanup_(prioq_freep) Prioq *q = NULL; + _cleanup_(set_freep) Set *s = NULL; unsigned previous = 0, i; - int r; + struct test *t; srand(0); - q = prioq_new(test_compare); - assert_se(q); - - s = set_new(&test_hash_ops); - assert_se(s); + assert_se(q = prioq_new(test_compare)); + assert_se(s = set_new(&test_hash_ops)); for (i = 0; i < SET_SIZE; i++) { - struct test *t; - - t = new0(struct test, 1); - assert_se(t); + assert_se(t = new0(struct test, 1)); t->value = (unsigned) rand(); - r = prioq_put(q, t, &t->idx); - assert_se(r >= 0); + assert_se(prioq_put(q, t, &t->idx) >= 0); - if (i % 4 == 0) { - r = set_consume(s, t); - assert_se(r >= 0); - } + if (i % 4 == 0) + assert_se(set_consume(s, t) >= 0); } - for (;;) { - struct test *t; - - t = set_steal_first(s); - if (!t) - break; - - r = prioq_remove(q, t, &t->idx); - assert_se(r > 0); + while ((t = set_steal_first(s))) { + assert_se(prioq_remove(q, t, &t->idx) == 1); + assert_se(prioq_remove(q, t, &t->idx) == 0); free(t); } for (i = 0; i < SET_SIZE * 3 / 4; i++) { - struct test *t; - assert_se(prioq_size(q) == (SET_SIZE * 3 / 4) - i); - t = prioq_pop(q); - assert_se(t); - + assert_se(t = prioq_pop(q)); assert_se(previous <= t->value); + previous = t->value; free(t); } assert_se(prioq_isempty(q)); - prioq_free(q); - assert_se(set_isempty(s)); - set_free(s); } int main(int argc, char* argv[]) { From cd86deefa141b719cde080146ed6027fa0786a52 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 Oct 2018 22:27:30 +0900 Subject: [PATCH 3/7] util: fix segfault in prioq_remove() with empty Prioq object --- src/basic/prioq.c | 3 +++ src/test/test-prioq.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/basic/prioq.c b/src/basic/prioq.c index ef28a086d1..e816b2c62a 100644 --- a/src/basic/prioq.c +++ b/src/basic/prioq.c @@ -211,6 +211,9 @@ _pure_ static struct prioq_item* find_item(Prioq *q, void *data, unsigned *idx) assert(q); + if (q->n_items <= 0) + return NULL; + if (idx) { if (*idx == PRIOQ_IDX_NULL || *idx > q->n_items) diff --git a/src/test/test-prioq.c b/src/test/test-prioq.c index 8aff66978e..b2ee70bca6 100644 --- a/src/test/test-prioq.c +++ b/src/test/test-prioq.c @@ -86,6 +86,7 @@ static void test_struct(void) { while ((t = set_steal_first(s))) { assert_se(prioq_remove(q, t, &t->idx) == 1); assert_se(prioq_remove(q, t, &t->idx) == 0); + assert_se(prioq_remove(q, t, NULL) == 0); free(t); } @@ -94,6 +95,8 @@ static void test_struct(void) { assert_se(prioq_size(q) == (SET_SIZE * 3 / 4) - i); assert_se(t = prioq_pop(q)); + assert_se(prioq_remove(q, t, &t->idx) == 0); + assert_se(prioq_remove(q, t, NULL) == 0); assert_se(previous <= t->value); previous = t->value; From 9f934223b2051bd5aa5c97fa0bf9eb777a711e90 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 Oct 2018 23:35:22 +0900 Subject: [PATCH 4/7] prioq: add one more assertion --- src/basic/prioq.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/basic/prioq.c b/src/basic/prioq.c index e816b2c62a..1e4d8d1b2f 100644 --- a/src/basic/prioq.c +++ b/src/basic/prioq.c @@ -88,6 +88,7 @@ static void swap(Prioq *q, unsigned j, unsigned k) { static unsigned shuffle_up(Prioq *q, unsigned idx) { assert(q); + assert(idx < q->n_items); while (idx > 0) { unsigned k; From e6e637a11a6c62eff31d36f5fc4b49c2a10c7ea8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Oct 2018 00:38:52 +0900 Subject: [PATCH 5/7] prioq: fix index range check --- src/basic/prioq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/prioq.c b/src/basic/prioq.c index 1e4d8d1b2f..8dcefa1a65 100644 --- a/src/basic/prioq.c +++ b/src/basic/prioq.c @@ -217,7 +217,7 @@ _pure_ static struct prioq_item* find_item(Prioq *q, void *data, unsigned *idx) if (idx) { if (*idx == PRIOQ_IDX_NULL || - *idx > q->n_items) + *idx >= q->n_items) return NULL; i = q->items + *idx; From 0cb3b295ec82357162fe625d82b439ecbbe5f2a2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Oct 2018 00:47:50 +0900 Subject: [PATCH 6/7] prioq: use structrued initializer --- src/basic/prioq.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/basic/prioq.c b/src/basic/prioq.c index 8dcefa1a65..4ef4eaf0cd 100644 --- a/src/basic/prioq.c +++ b/src/basic/prioq.c @@ -32,11 +32,14 @@ struct Prioq { Prioq *prioq_new(compare_func_t compare_func) { Prioq *q; - q = new0(Prioq, 1); + q = new(Prioq, 1); if (!q) return q; - q->compare_func = compare_func; + *q = (Prioq) { + .compare_func = compare_func, + }; + return q; } From 5023c62efdd5749f3edcaa3055cd17ba8a05af13 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 17 Oct 2018 01:18:05 +0900 Subject: [PATCH 7/7] test: add one more test for prioq_remove() This adds a testcase for e6e637a11a6c62eff31d36f5fc4b49c2a10c7ea8. --- src/test/test-prioq.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/test-prioq.c b/src/test/test-prioq.c index b2ee70bca6..a93d37d460 100644 --- a/src/test/test-prioq.c +++ b/src/test/test-prioq.c @@ -16,7 +16,7 @@ static int unsigned_compare(const unsigned *a, const unsigned *b) { static void test_unsigned(void) { _cleanup_(prioq_freep) Prioq *q = NULL; - unsigned buffer[SET_SIZE], i, u; + unsigned buffer[SET_SIZE], i, u, n; srand(0); @@ -26,6 +26,9 @@ static void test_unsigned(void) { u = (unsigned) rand(); buffer[i] = u; assert_se(prioq_put(q, UINT_TO_PTR(u), NULL) >= 0); + + n = prioq_size(q); + assert_se(prioq_remove(q, UINT_TO_PTR(u), &n) == 0); } typesafe_qsort(buffer, ELEMENTSOF(buffer), unsigned_compare);