Re: [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@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.

+       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