On 15-Sep-2023, at 8:00 PM, Reza Arbab [off-list ref] wrote:

Hi Athira,

On Thu, Sep 14, 2023 at 10:02:04PM +0530, Athira Rajeev wrote: 

quoted

+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.

quoted

+ }
+
+ 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.

Hi Reza,

Sure, Change looks good. Thanks for the change and fixup.

Thanks
Athira

quoted

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@1") == addr1);
+ assert(dt_find_by_name(root, "node0_1@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.

quoted

+ 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