| Message ID | 20230216080859.19161-1-atrajeev@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V3,1/3] core/device: Add function to return child node using name at substring "@" | expand |
> On 16-Feb-2023, at 1:38 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote: > > Add a function dt_find_by_name_substr() that returns the child node if > it matches till first occurence at "@" of a given name, otherwise NULL. > This is helpful for cases with node name like: "name@addr". In > scenarios where nodes are added with "name@addr" format and if the > value of "addr" is not known, that node can't be matched with node > name or addr. Hence matching with substring as node name will return > the expected result. Patch adds dt_find_by_name_substr() function > and testcase for the same in core/test/run-device.c > > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> > --- > Changelog: > v2 -> v3: > - After review comments from Mahesh, fixed the code > to consider string upto "@" for both input node name > as well as child node name. V2 version was comparing > input node name and child node name upto string length > of child name. But this will return wrong node if input > name is larger than child name. Because it will match > as substring for child name. > https://lists.ozlabs.org/pipermail/skiboot/2023-January/018596.html Hi, Looking for review comments for this V3 patch set. Please share your feedback. Thanks Athira > > v1 -> v2: > - Addressed review comment from Dan to update > the utility funtion to search and compare > upto "@". Renamed it as dt_find_by_name_substr. > > core/device.c | 35 +++++++++++++++++++++++++++++++++++ > core/test/run-device.c | 15 +++++++++++++++ > include/device.h | 3 +++ > 3 files changed, 53 insertions(+) > > diff --git a/core/device.c b/core/device.c > index 2de37c74..26562235 100644 > --- a/core/device.c > +++ b/core/device.c > @@ -395,6 +395,41 @@ struct dt_node *dt_find_by_name(struct dt_node *root, const char *name) > } > > > +struct dt_node *dt_find_by_name_substr(struct dt_node *root, const char *name) > +{ > + struct dt_node *child, *match; > + char *node, *child_node = NULL; > + > + node = malloc(strlen(name) + 1); > + if (!node) > + return NULL; > + memcpy(node, name, strlen(name)); > + node[strlen(name)] = '\0'; > + node = strtok(node, "@"); > + list_for_each(&root->children, child, list) { > + child_node = malloc(strlen(child->name) + 1); > + if (!child_node) > + goto err; > + memcpy(child_node, child->name, strlen(child->name)); > + child_node[strlen(child->name)] = '\0'; > + child_node = strtok(child_node, "@"); > + if (!strcmp(child_node, node)) { > + free(child_node); > + free(node); > + return child; > + } > + > + match = dt_find_by_name_substr(child, name); > + if (match) > + return match; > + } > + > + free(child_node); > +err: > + free(node); > + return NULL; > +} > + > struct dt_node *dt_new_check(struct dt_node *parent, const char *name) > { > struct dt_node *node = dt_find_by_name(parent, name); > diff --git a/core/test/run-device.c b/core/test/run-device.c > index 4a12382b..6997452e 100644 > --- a/core/test/run-device.c > +++ b/core/test/run-device.c > @@ -466,6 +466,21 @@ 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_substr */ > + 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_substr(root, "node@1") == addr1); > + assert(dt_find_by_name_substr(root, "node0_1@2") == addr2); > + assert(dt_find_by_name_substr(root, "node0_") == NULL); > + assert(dt_find_by_name_substr(root, "node0_1") == addr2); > + assert(dt_find_by_name_substr(root, "node0@") == NULL); > + assert(dt_find_by_name_substr(root, "node0_@") == NULL); > + dt_free(root); > + > return 0; > } > > diff --git a/include/device.h b/include/device.h > index 93fb90ff..b6a1a813 100644 > --- a/include/device.h > +++ b/include/device.h > @@ -184,6 +184,9 @@ struct dt_node *dt_find_by_path(struct dt_node *root, const char *path); > /* Find a child node by name */ > struct dt_node *dt_find_by_name(struct dt_node *root, const char *name); > > +/* Find a child node by name and substring */ > +struct dt_node *dt_find_by_name_substr(struct dt_node *root, const char *name); > + > /* Find a node by phandle */ > struct dt_node *dt_find_by_phandle(struct dt_node *root, u32 phandle); > > -- > 2.27.0 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On 2023-02-16 13:38:57 Thu, Athira Rajeev wrote: > Add a function dt_find_by_name_substr() that returns the child node if > it matches till first occurence at "@" of a given name, otherwise NULL. > This is helpful for cases with node name like: "name@addr". In > scenarios where nodes are added with "name@addr" format and if the > value of "addr" is not known, that node can't be matched with node > name or addr. Hence matching with substring as node name will return > the expected result. Patch adds dt_find_by_name_substr() function > and testcase for the same in core/test/run-device.c > > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> > --- [...] > diff --git a/core/device.c b/core/device.c > index 2de37c74..26562235 100644 > --- a/core/device.c > +++ b/core/device.c > @@ -395,6 +395,41 @@ struct dt_node *dt_find_by_name(struct dt_node *root, const char *name) > } > > > +struct dt_node *dt_find_by_name_substr(struct dt_node *root, const char *name) > +{ > + struct dt_node *child, *match; > + char *node, *child_node = NULL; > + > + node = malloc(strlen(name) + 1); > + if (!node) > + return NULL; > + memcpy(node, name, strlen(name)); > + node[strlen(name)] = '\0'; You may want to use strdup() which does exactly what you trying to do: node = strdup(name); if (!node) return NULL; > + node = strtok(node, "@"); > + list_for_each(&root->children, child, list) { > + child_node = malloc(strlen(child->name) + 1); > + if (!child_node) > + goto err; > + memcpy(child_node, child->name, strlen(child->name)); > + child_node[strlen(child->name)] = '\0'; same here ^^^ Rest looks fine to me. Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> Thanks, -Mahesh.
diff --git a/core/device.c b/core/device.c index 2de37c74..26562235 100644 --- a/core/device.c +++ b/core/device.c @@ -395,6 +395,41 @@ struct dt_node *dt_find_by_name(struct dt_node *root, const char *name) } +struct dt_node *dt_find_by_name_substr(struct dt_node *root, const char *name) +{ + struct dt_node *child, *match; + char *node, *child_node = NULL; + + node = malloc(strlen(name) + 1); + if (!node) + return NULL; + memcpy(node, name, strlen(name)); + node[strlen(name)] = '\0'; + node = strtok(node, "@"); + list_for_each(&root->children, child, list) { + child_node = malloc(strlen(child->name) + 1); + if (!child_node) + goto err; + memcpy(child_node, child->name, strlen(child->name)); + child_node[strlen(child->name)] = '\0'; + child_node = strtok(child_node, "@"); + if (!strcmp(child_node, node)) { + free(child_node); + free(node); + return child; + } + + match = dt_find_by_name_substr(child, name); + if (match) + return match; + } + + free(child_node); +err: + free(node); + return NULL; +} + struct dt_node *dt_new_check(struct dt_node *parent, const char *name) { struct dt_node *node = dt_find_by_name(parent, name); diff --git a/core/test/run-device.c b/core/test/run-device.c index 4a12382b..6997452e 100644 --- a/core/test/run-device.c +++ b/core/test/run-device.c @@ -466,6 +466,21 @@ 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_substr */ + 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_substr(root, "node@1") == addr1); + assert(dt_find_by_name_substr(root, "node0_1@2") == addr2); + assert(dt_find_by_name_substr(root, "node0_") == NULL); + assert(dt_find_by_name_substr(root, "node0_1") == addr2); + assert(dt_find_by_name_substr(root, "node0@") == NULL); + assert(dt_find_by_name_substr(root, "node0_@") == NULL); + dt_free(root); + return 0; } diff --git a/include/device.h b/include/device.h index 93fb90ff..b6a1a813 100644 --- a/include/device.h +++ b/include/device.h @@ -184,6 +184,9 @@ struct dt_node *dt_find_by_path(struct dt_node *root, const char *path); /* Find a child node by name */ struct dt_node *dt_find_by_name(struct dt_node *root, const char *name); +/* Find a child node by name and substring */ +struct dt_node *dt_find_by_name_substr(struct dt_node *root, const char *name); + /* Find a node by phandle */ struct dt_node *dt_find_by_phandle(struct dt_node *root, u32 phandle);
Add a function dt_find_by_name_substr() that returns the child node if it matches till first occurence at "@" of a given name, otherwise NULL. This is helpful for cases with node name like: "name@addr". In scenarios where nodes are added with "name@addr" format and if the value of "addr" is not known, that node can't be matched with node name or addr. Hence matching with substring as node name will return the expected result. Patch adds dt_find_by_name_substr() function and testcase for the same in core/test/run-device.c Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> --- Changelog: v2 -> v3: - After review comments from Mahesh, fixed the code to consider string upto "@" for both input node name as well as child node name. V2 version was comparing input node name and child node name upto string length of child name. But this will return wrong node if input name is larger than child name. Because it will match as substring for child name. https://lists.ozlabs.org/pipermail/skiboot/2023-January/018596.html v1 -> v2: - Addressed review comment from Dan to update the utility funtion to search and compare upto "@". Renamed it as dt_find_by_name_substr. core/device.c | 35 +++++++++++++++++++++++++++++++++++ core/test/run-device.c | 15 +++++++++++++++ include/device.h | 3 +++ 3 files changed, 53 insertions(+)