> On 15-Sep-2023, at 8:00 PM, Reza Arbab <ar...@linux.ibm.com> wrote:
>
> 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.
Hi Reza,
Sure, Change looks good. Thanks for the change and fixup.
Thanks
Athira
>
>> 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
