From 4a5567071d9229595c1fbcf769a32b0bce1a7e8d Mon Sep 17 00:00:00 2001 From: Subrata Banik Date: Tue, 27 Jan 2026 17:13:57 +0000 Subject: [PATCH] Revert "commonlib/list: Support circular list" This reverts commit c4be70f6ff5379bcaeb7f0d2f17b140c70fc12cf. Reason for revert: The CL caused a hang in Depthcharge on Google/Quartz. BUG=b:479143030 TEST=Verify boot on Google/Quartz. Change-Id: I38087d0b2dd218dfb32a02c343b199708bb47d49 Signed-off-by: Kapil Porwal Reviewed-on: https://review.coreboot.org/c/coreboot/+/90956 Tested-by: build bot (Jenkins) --- src/commonlib/include/commonlib/list.h | 21 ++------ src/commonlib/list.c | 24 ++------- tests/commonlib/list-test.c | 70 ++++++++------------------ 3 files changed, 31 insertions(+), 84 deletions(-) diff --git a/src/commonlib/include/commonlib/list.h b/src/commonlib/include/commonlib/list.h index 7cd3460f8a..970d68ea49 100644 --- a/src/commonlib/include/commonlib/list.h +++ b/src/commonlib/include/commonlib/list.h @@ -17,25 +17,14 @@ 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); -// Append the node to the end of the list. +// Appends the node to the end of the list. void list_append(struct list_node *node, struct list_node *head); -/* - * 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, \ +#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, \ typeof(*(ptr)), member)) #endif /* __COMMONLIB_LIST_H__ */ diff --git a/src/commonlib/list.c b/src/commonlib/list.c index 9cbb554b41..b1030c8263 100644 --- a/src/commonlib/list.c +++ b/src/commonlib/list.c @@ -1,22 +1,10 @@ /* 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) @@ -25,9 +13,6 @@ 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; @@ -37,8 +22,6 @@ 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; @@ -48,7 +31,8 @@ void list_insert_before(struct list_node *node, struct list_node *before) void list_append(struct list_node *node, struct list_node *head) { - list_init(head); - /* With a circular list, we just need to insert before the head. */ - list_insert_before(node, head); + while (head->next) + head = head->next; + + list_insert_after(node, head); } diff --git a/tests/commonlib/list-test.c b/tests/commonlib/list-test.c index eeeeded059..4ca48a468f 100644 --- a/tests/commonlib/list-test.c +++ b/tests/commonlib/list-test.c @@ -1,10 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include +#include #include #include -#include -#include +#include struct test_container { int value; @@ -12,20 +11,10 @@ struct test_container { struct list_node list_node; }; -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) +void test_list_insert_after(void **state) { int i = 0; - struct list_node head = {}; + struct list_node root = { .prev = NULL, .next = NULL }; 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)); @@ -40,11 +29,11 @@ static void test_list_insert_after(void **state) c2->value = values[1]; c3->value = values[2]; - list_insert_after(&c1->list_node, &head); + list_insert_after(&c1->list_node, &root); list_insert_after(&c2->list_node, &c1->list_node); list_insert_after(&c3->list_node, &c2->list_node); - list_for_each(ptr, head, list_node) { + list_for_each(ptr, root, list_node) { assert_int_equal(values[i], ptr->value); i++; } @@ -56,10 +45,10 @@ static void test_list_insert_after(void **state) free(c1); } -static void test_list_insert_before(void **state) +void test_list_insert_before(void **state) { int i = 0; - struct list_node head = {}; + struct list_node root = { .prev = NULL, .next = NULL }; 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)); @@ -74,11 +63,12 @@ static void test_list_insert_before(void **state) c2->value = values[1]; c3->value = values[2]; - list_insert_after(&c3->list_node, &head); + list_insert_after(&c3->list_node, &root); list_insert_before(&c2->list_node, &c3->list_node); list_insert_before(&c1->list_node, &c2->list_node); - list_for_each(ptr, head, list_node) { + + list_for_each(ptr, root, list_node) { assert_int_equal(values[i], ptr->value); i++; } @@ -90,27 +80,19 @@ static void test_list_insert_before(void **state) free(c1); } -static void test_list_insert_before_head(void **state) +void test_list_remove(void **state) { - 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 list_node root = { .prev = NULL, .next = NULL }; 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, &head); + list_insert_after(&c1->list_node, &root); list_insert_after(&c2->list_node, &c1->list_node); len = 0; - list_for_each(ptr, head, list_node) { + list_for_each(ptr, root, list_node) { len++; } assert_int_equal(2, len); @@ -118,14 +100,14 @@ static void test_list_remove(void **state) list_remove(&c1->list_node); len = 0; - list_for_each(ptr, head, list_node) { + list_for_each(ptr, root, list_node) { len++; } assert_int_equal(1, len); list_remove(&c2->list_node); len = 0; - list_for_each(ptr, head, list_node) { + list_for_each(ptr, root, list_node) { len++; } assert_int_equal(0, len); @@ -134,26 +116,20 @@ static void test_list_remove(void **state) free(c1); } -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) +void test_list_append(void **state) { size_t idx; struct test_container *node; - struct list_node head = {}; + struct list_node root = {}; struct test_container nodes[] = { {1}, {2}, {3} }; for (idx = 0; idx < ARRAY_SIZE(nodes); ++idx) - list_append(&nodes[idx].list_node, &head); + list_append(&nodes[idx].list_node, &root); idx = 0; - list_for_each(node, head, list_node) { + list_for_each(node, root, list_node) { assert_ptr_equal(node, &nodes[idx]); idx++; } @@ -162,14 +138,12 @@ static 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); }