[Skiboot] [PATCH 1/3] core/device: Add function to return child node using name at substring "@"

[Reza Arbab]

Hi Athira,

On Thu, Sep 14, 2023 at 10:02:04PM +0530, Athira Rajeev wrote: 
>+struct dt_node *dt_find_by_name_before_addr(struct dt_node *root, const char *name)
>+{
>+	struct dt_node *child, *match;
>+	char *child_node = NULL;
>+
>+	list_for_each(&root->children, child, list) {
>+		child_node = strdup(child->name);
>+		if (!child_node)
>+			goto err;
>+		child_node = strtok(child_node, "@");
>+		if (!strcmp(child_node, name)) {
>+			free(child_node);
>+			return child;
>+		}
>+
>+		match = dt_find_by_name_before_addr(child, name);
>+		if (match)
>+			return match;

When the function returns on this line, child_node is not freed.

>+	}
>+
>+	free(child_node);
>+err:
>+	return NULL;
>+}

I took at stab at moving free(child_node) inside the loop, and ended up 
with this:

struct dt_node *dt_find_by_name_before_addr(struct dt_node *root, const char *name)
{
	struct dt_node *child, *match = NULL;
	char *child_name = NULL;

	list_for_each(&root->children, child, list) {
		child_name = strdup(child->name);
		if (!child_name)
			return NULL;

		child_name = strtok(child_name, "@");
		if (!strcmp(child_name, name))
			match = child;
		else
			match = dt_find_by_name_before_addr(child, name);

		free(child_name);
		if (match)
			return match;
	}

	return NULL;
}

Does this seem okay to you? If you agree, no need to send another 
revision, I can just fixup during commit. Let me know.

>diff --git a/core/test/run-device.c b/core/test/run-device.c
>index 4a12382bb..fb7a7d2c0 100644
>--- a/core/test/run-device.c
>+++ b/core/test/run-device.c
>@@ -466,6 +466,20 @@ int main(void)
> 	new_prop_ph = dt_prop_get_u32(ut2, "something");
> 	assert(!(new_prop_ph == ev1_ph));
> 	dt_free(subtree);
>+
>+	/* Test dt_find_by_name_before_addr */
>+	root = dt_new_root("");
>+	addr1 = dt_new_addr(root, "node", 0x1);
>+	addr2 = dt_new_addr(root, "node0_1", 0x2);
>+	assert(dt_find_by_name(root, "node at 1") == addr1);
>+	assert(dt_find_by_name(root, "node0_1 at 2") == addr2);
>+	assert(dt_find_by_name_before_addr(root, "node") == addr1);
>+	assert(dt_find_by_name_before_addr(root, "node0_") == NULL);

This line appears twice. As above, can fix during commit, so no need for 
a new patch.

>+	assert(dt_find_by_name_before_addr(root, "node0_1") == addr2);
>+	assert(dt_find_by_name_before_addr(root, "node0") == NULL);
>+	assert(dt_find_by_name_before_addr(root, "node0_") == NULL);
>+	dt_free(root);
>+
> 	return 0;
> }
>

-- 
Reza Arbab