commonlib/device_tree: Utilize list_move() in dt_copy_subtree()
In dt_copy_subtree(), the device_tree_node copying
*dst_node = *src_node;
doesn't work correctly for circular linked lists [1], because the 'next'
pointer of the last element isn't modified to point to the dst head.
As the only public caller of dt_copy_subtree() is dt_apply_overlay(),
and the dt_apply_overlay() function comment already explicitly disallows
'overlay' accesses after the call, fix the problem by utilizing
list_move() for copying device tree node properties and children.
Also add a new test case test_dt_apply_overlay.
[1] commit 23c41622a9 ("commonlib/list: Change to circular list")
BUG=b:434080284
TEST=emerge-rauru coreboot libpayload
BRANCH=none
Change-Id: I166ab74c9de67330d52f94e92b5d7ce5ddefa82b
Signed-off-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/91558
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Reviewed-by: Kapil Porwal <kapilporwal@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Reviewed-by: Jakub "Kuba" Czapiga <czapiga@google.com>
This commit is contained in:
parent
89048780c0
commit
1e1b63c23b
3 changed files with 128 additions and 1 deletions
|
|
@ -2001,7 +2001,10 @@ static void dt_copy_subtree(struct device_tree_node *dst,
|
|||
|
||||
if (!dst_node) {
|
||||
dst_node = alloc_node();
|
||||
*dst_node = *src_node;
|
||||
dst_node->name = src_node->name;
|
||||
dst_node->phandle = src_node->phandle;
|
||||
list_move(&dst_node->properties, &src_node->properties);
|
||||
list_move(&dst_node->children, &src_node->children);
|
||||
list_insert_after(&dst_node->list_node, &dst->children);
|
||||
} else {
|
||||
dt_copy_subtree(dst_node, src_node, upd);
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@ tests-y += device_tree-test
|
|||
|
||||
device_tree-test-srcs += tests/commonlib/device_tree-test.c
|
||||
device_tree-test-srcs += tests/stubs/console.c
|
||||
device_tree-test-srcs += src/commonlib/bsd/string.c
|
||||
device_tree-test-srcs += src/commonlib/device_tree.c
|
||||
device_tree-test-srcs += src/commonlib/list.c
|
||||
device_tree-test-syssrcs += tests/helpers/file.c
|
||||
|
||||
list-test-srcs += tests/commonlib/list-test.c
|
||||
|
|
|
|||
|
|
@ -10,6 +10,21 @@
|
|||
#include <string.h>
|
||||
#include <tests/test.h>
|
||||
|
||||
static struct device_tree *new_device_tree(void)
|
||||
{
|
||||
struct device_tree *tree = xzalloc(sizeof(*tree));
|
||||
tree->root = xzalloc(sizeof(*tree->root));
|
||||
tree->root->name = "root";
|
||||
return tree;
|
||||
}
|
||||
|
||||
static void free_device_tree(struct device_tree *tree)
|
||||
{
|
||||
assert_non_null(tree);
|
||||
free(tree->root);
|
||||
free(tree);
|
||||
}
|
||||
|
||||
static int setup_device_tree_test_group(void **state)
|
||||
{
|
||||
/*
|
||||
|
|
@ -117,6 +132,112 @@ static void test_fdt_read_reg_prop(void **state)
|
|||
assert_int_equal(0x00010000, regions[0].size);
|
||||
}
|
||||
|
||||
static void test_dt_apply_overlay(void **state)
|
||||
{
|
||||
struct device_tree *base = new_device_tree();
|
||||
struct device_tree *overlay = new_device_tree();
|
||||
assert_non_null(base);
|
||||
assert_non_null(overlay);
|
||||
|
||||
/* Add content to base tree BEFORE overlay */
|
||||
struct device_tree_node *base_existing = dt_find_node_by_path(
|
||||
base, "/preexisting", NULL, NULL, 1);
|
||||
assert_non_null(base_existing);
|
||||
dt_add_string_prop(base_existing, "base_prop", "base_val");
|
||||
|
||||
/*
|
||||
* Overlay:
|
||||
*
|
||||
* /fragment@0 {
|
||||
* target-path = "/";
|
||||
* __overlay__ {
|
||||
* new_parent {
|
||||
* parent_prop = "parent_val";
|
||||
* new_child {
|
||||
* child_prop = "child_val";
|
||||
* };
|
||||
* };
|
||||
* };
|
||||
* };
|
||||
*/
|
||||
struct device_tree_node *frag = dt_find_node_by_path(
|
||||
overlay, "/fragment@0", NULL, NULL, 1);
|
||||
assert_non_null(frag);
|
||||
dt_add_string_prop(frag, "target-path", "/");
|
||||
|
||||
/* Path to new_parent in overlay. */
|
||||
static const char *const parent_path[] = {
|
||||
"fragment@0",
|
||||
"__overlay__",
|
||||
"new_parent",
|
||||
NULL,
|
||||
};
|
||||
struct device_tree_node *ovl_parent = dt_find_node(
|
||||
overlay->root, parent_path, NULL, NULL, 1);
|
||||
assert_non_null(ovl_parent);
|
||||
dt_add_string_prop(ovl_parent, "parent_prop", "parent_val");
|
||||
|
||||
/* Path to new_child in overlay. */
|
||||
static const char *const child_path[] = {
|
||||
"fragment@0",
|
||||
"__overlay__",
|
||||
"new_parent",
|
||||
"new_child",
|
||||
NULL,
|
||||
};
|
||||
struct device_tree_node *ovl_child = dt_find_node(
|
||||
overlay->root, child_path, NULL, NULL, 1);
|
||||
assert_non_null(ovl_child);
|
||||
dt_add_string_prop(ovl_child, "child_prop", "child_val");
|
||||
|
||||
int ret = dt_apply_overlay(base, overlay);
|
||||
assert_int_equal(ret, 0);
|
||||
|
||||
/* Check base tree for the preexisting node. */
|
||||
struct device_tree_node *check_preexisting = dt_find_node_by_path(
|
||||
base, "/preexisting", NULL, NULL, 0);
|
||||
assert_non_null(check_preexisting);
|
||||
const char *preexisting_val = dt_find_string_prop(check_preexisting, "base_prop");
|
||||
assert_string_equal(preexisting_val, "base_val");
|
||||
|
||||
/* Check base tree for the new structure. */
|
||||
struct device_tree_node *new_parent = dt_find_node_by_path(
|
||||
base, "/new_parent", NULL, NULL, 0);
|
||||
assert_non_null(new_parent);
|
||||
const char *parent_val = dt_find_string_prop(new_parent, "parent_prop");
|
||||
assert_string_equal(parent_val, "parent_val");
|
||||
|
||||
struct device_tree_node *new_child = dt_find_node_by_path(
|
||||
base, "/new_parent/new_child", NULL, NULL, 0);
|
||||
assert_non_null(new_child);
|
||||
const char *child_val = dt_find_string_prop(new_child, "child_prop");
|
||||
assert_string_equal(child_val, "child_val");
|
||||
|
||||
/* Check that list_for_each works on the new nodes. */
|
||||
struct device_tree_node *node;
|
||||
int child_count = 0;
|
||||
list_for_each(node, new_parent->children, list_node) {
|
||||
child_count++;
|
||||
assert_ptr_equal(node, new_child);
|
||||
}
|
||||
assert_int_equal(child_count, 1);
|
||||
|
||||
struct device_tree_property *prop_iter;
|
||||
int parent_prop_count = 0;
|
||||
list_for_each(prop_iter, new_parent->properties, list_node) {
|
||||
parent_prop_count++;
|
||||
}
|
||||
assert_int_equal(parent_prop_count, 1);
|
||||
|
||||
int child_prop_count = 0;
|
||||
list_for_each(prop_iter, new_child->properties, list_node) {
|
||||
child_prop_count++;
|
||||
}
|
||||
assert_int_equal(child_prop_count, 1);
|
||||
|
||||
free_device_tree(base);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
const struct CMUnitTest tests[] = {
|
||||
|
|
@ -125,6 +246,7 @@ int main(void)
|
|||
cmocka_unit_test(test_fdt_find_node_by_alias),
|
||||
cmocka_unit_test(test_fdt_find_prop_in_node),
|
||||
cmocka_unit_test(test_fdt_read_reg_prop),
|
||||
cmocka_unit_test(test_dt_apply_overlay),
|
||||
};
|
||||
|
||||
return cb_run_group_tests(tests, setup_device_tree_test_group,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue