diff --git a/src/commonlib/include/commonlib/list.h b/src/commonlib/include/commonlib/list.h index 970d68ea49..7cd3460f8a 100644 --- a/src/commonlib/include/commonlib/list.h +++ b/src/commonlib/include/commonlib/list.h @@ -17,14 +17,25 @@ void list_remove(struct list_node *node); // Insert list_node node after list_node after in a doubly linked list. void list_insert_after(struct list_node *node, struct list_node *after); // Insert list_node node before list_node before in a doubly linked list. +// `before` must not be the placeholder head node. void list_insert_before(struct list_node *node, struct list_node *before); -// Appends the node to the end of the list. +// Append the node to the end of the list. void list_append(struct list_node *node, struct list_node *head); -#define list_for_each(ptr, head, member) \ - for ((ptr) = container_of((head).next, typeof(*(ptr)), member); \ - (uintptr_t)ptr + (uintptr_t)offsetof(typeof(*(ptr)), member); \ - (ptr) = container_of((ptr)->member.next, \ +/* + * Explanation of `ptr` initialization: + * 1. head.next != NULL: This means the list isn't empty. As the implementation ensures that + * list_init() is called when the very first element is added, we can safely assume that + * the list is circular, and hence set `ptr` to the 1st element. + * 2. head.next == NULL: This means the list is empty, and list_init() hasn't been called. + * As the `head` arg might be const, we cannot simply call list_init() here. Instead, we set + * `ptr` to a special value such that `&(ptr->member) == &head`, causing the loop to + * terminate immediately. + */ +#define list_for_each(ptr, head, member) \ + for ((ptr) = container_of((head).next ?: &(head), typeof(*(ptr)), member); \ + (&((ptr)->member) != &(head)); \ + (ptr) = container_of((ptr)->member.next, \ typeof(*(ptr)), member)) #endif /* __COMMONLIB_LIST_H__ */ diff --git a/src/commonlib/list.c b/src/commonlib/list.c index b1030c8263..9cbb554b41 100644 --- a/src/commonlib/list.c +++ b/src/commonlib/list.c @@ -1,10 +1,22 @@ /* Taken from depthcharge: src/base/list.c */ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include #include +// Initialize a circular list, with `head` being a placeholder head node. +static void list_init(struct list_node *head) +{ + if (!head->next) { + assert(!head->prev); + head->next = head->prev = head; + } +} + void list_remove(struct list_node *node) { + /* Cannot remove the head node. */ + assert(node->prev && node->next); if (node->prev) node->prev->next = node->next; if (node->next) @@ -13,6 +25,9 @@ void list_remove(struct list_node *node) void list_insert_after(struct list_node *node, struct list_node *after) { + /* Check uninitialized head node. */ + if (after->prev == NULL) + list_init(after); node->next = after->next; node->prev = after; after->next = node; @@ -22,6 +37,8 @@ void list_insert_after(struct list_node *node, struct list_node *after) void list_insert_before(struct list_node *node, struct list_node *before) { + /* `before` cannot be an uninitialized head node. */ + assert(before->prev) node->prev = before->prev; node->next = before; before->prev = node; @@ -31,8 +48,7 @@ void list_insert_before(struct list_node *node, struct list_node *before) void list_append(struct list_node *node, struct list_node *head) { - while (head->next) - head = head->next; - - list_insert_after(node, head); + list_init(head); + /* With a circular list, we just need to insert before the head. */ + list_insert_before(node, head); } diff --git a/tests/commonlib/list-test.c b/tests/commonlib/list-test.c index 4ca48a468f..eeeeded059 100644 --- a/tests/commonlib/list-test.c +++ b/tests/commonlib/list-test.c @@ -1,9 +1,10 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include +#include #include #include -#include +#include +#include struct test_container { int value; @@ -11,10 +12,20 @@ struct test_container { struct list_node list_node; }; -void test_list_insert_after(void **state) +static void test_list_empty(void **state) +{ + struct list_node head = {}; + struct test_container *node; + + list_for_each(node, head, list_node) { + assert_true(false); + } +} + +static void test_list_insert_after(void **state) { int i = 0; - struct list_node root = { .prev = NULL, .next = NULL }; + struct list_node head = {}; struct test_container *c1 = (struct test_container *)malloc(sizeof(*c1)); struct test_container *c2 = (struct test_container *)malloc(sizeof(*c2)); struct test_container *c3 = (struct test_container *)malloc(sizeof(*c2)); @@ -29,11 +40,11 @@ void test_list_insert_after(void **state) c2->value = values[1]; c3->value = values[2]; - list_insert_after(&c1->list_node, &root); + list_insert_after(&c1->list_node, &head); list_insert_after(&c2->list_node, &c1->list_node); list_insert_after(&c3->list_node, &c2->list_node); - list_for_each(ptr, root, list_node) { + list_for_each(ptr, head, list_node) { assert_int_equal(values[i], ptr->value); i++; } @@ -45,10 +56,10 @@ void test_list_insert_after(void **state) free(c1); } -void test_list_insert_before(void **state) +static void test_list_insert_before(void **state) { int i = 0; - struct list_node root = { .prev = NULL, .next = NULL }; + struct list_node head = {}; struct test_container *c1 = (struct test_container *)malloc(sizeof(*c1)); struct test_container *c2 = (struct test_container *)malloc(sizeof(*c2)); struct test_container *c3 = (struct test_container *)malloc(sizeof(*c2)); @@ -63,12 +74,11 @@ void test_list_insert_before(void **state) c2->value = values[1]; c3->value = values[2]; - list_insert_after(&c3->list_node, &root); + list_insert_after(&c3->list_node, &head); list_insert_before(&c2->list_node, &c3->list_node); list_insert_before(&c1->list_node, &c2->list_node); - - list_for_each(ptr, root, list_node) { + list_for_each(ptr, head, list_node) { assert_int_equal(values[i], ptr->value); i++; } @@ -80,19 +90,27 @@ void test_list_insert_before(void **state) free(c1); } -void test_list_remove(void **state) +static void test_list_insert_before_head(void **state) { - struct list_node root = { .prev = NULL, .next = NULL }; + struct list_node head = {}; + struct test_container c = {}; + + expect_assert_failure(list_insert_before(&c.list_node, &head)); +} + +static void test_list_remove(void **state) +{ + struct list_node head = {}; struct test_container *c1 = (struct test_container *)malloc(sizeof(*c1)); struct test_container *c2 = (struct test_container *)malloc(sizeof(*c2)); struct test_container *ptr; int len; - list_insert_after(&c1->list_node, &root); + list_insert_after(&c1->list_node, &head); list_insert_after(&c2->list_node, &c1->list_node); len = 0; - list_for_each(ptr, root, list_node) { + list_for_each(ptr, head, list_node) { len++; } assert_int_equal(2, len); @@ -100,14 +118,14 @@ void test_list_remove(void **state) list_remove(&c1->list_node); len = 0; - list_for_each(ptr, root, list_node) { + list_for_each(ptr, head, list_node) { len++; } assert_int_equal(1, len); list_remove(&c2->list_node); len = 0; - list_for_each(ptr, root, list_node) { + list_for_each(ptr, head, list_node) { len++; } assert_int_equal(0, len); @@ -116,20 +134,26 @@ void test_list_remove(void **state) free(c1); } -void test_list_append(void **state) +static void test_list_remove_head(void **state) +{ + struct list_node head = {}; + expect_assert_failure(list_remove(&head)); +} + +static void test_list_append(void **state) { size_t idx; struct test_container *node; - struct list_node root = {}; + struct list_node head = {}; struct test_container nodes[] = { {1}, {2}, {3} }; for (idx = 0; idx < ARRAY_SIZE(nodes); ++idx) - list_append(&nodes[idx].list_node, &root); + list_append(&nodes[idx].list_node, &head); idx = 0; - list_for_each(node, root, list_node) { + list_for_each(node, head, list_node) { assert_ptr_equal(node, &nodes[idx]); idx++; } @@ -138,12 +162,14 @@ void test_list_append(void **state) int main(void) { const struct CMUnitTest tests[] = { + cmocka_unit_test(test_list_empty), cmocka_unit_test(test_list_insert_after), cmocka_unit_test(test_list_insert_before), + cmocka_unit_test(test_list_insert_before_head), cmocka_unit_test(test_list_remove), + cmocka_unit_test(test_list_remove_head), cmocka_unit_test(test_list_append), }; - return cb_run_group_tests(tests, NULL, NULL); }