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

[Mahesh J Salgaonkar]

On 2023-01-18 11:14:50 Wed, 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 at addr". In
> scenarios where nodes are added with "name at 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 at linux.vnet.ibm.com>
> ---
> Changelog:
> 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          | 18 ++++++++++++++++++
>  core/test/run-device.c | 11 +++++++++++
>  include/device.h       |  3 +++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/core/device.c b/core/device.c
> index 2de37c74..df3a5775 100644
> --- a/core/device.c
> +++ b/core/device.c
> @@ -395,6 +395,24 @@ 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 *pos;
> +
> +	list_for_each(&root->children, child, list) {
> +		pos = strchr(child->name, '@');
> +		if (!strncmp(child->name, name, pos - child->name))

Shouldn't we care about string length of substring to be checked before
comparision ? The code assumes that it is always within the limit of
position of '@' in node name string. Hence, it returns a wrong node
whose name partially matches with substring passed.

e.g.
With following two nodes in deviec tree (as per your test):
/node at 1
/node0_1 at 2

the substring 'node0', 'node0@' and 'node0_@' all matches with 'node at 1' device
tree node.
Is this expected ?

Also, what do you expect dt_find_by_name_substr() to return for string
like 'node0' and 'node0_' ? NULL or node '/node0_1 at 2' ?

> +			return child;
> +
> +		match = dt_find_by_name_substr(child, name);
> +		if (match)
> +			return match;
> +	}
> +
> +	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..0e463e58 100644
> --- a/core/test/run-device.c
> +++ b/core/test/run-device.c
> @@ -466,6 +466,17 @@ 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 at 1") == addr1);
> +	assert(dt_find_by_name(root, "node0_1 at 2") == addr2);
> +	assert(dt_find_by_name_substr(root, "node at 1") == addr1);
> +	assert(dt_find_by_name_substr(root, "node0_1 at 2") == addr2);

Below additional tests are failing:

	assert(dt_find_by_name_substr(root, "node0@") == NULL);
	assert(dt_find_by_name_substr(root, "node0_@") == NULL);

Maybe we should add few more test checks for "node0" and "node0_" as well.

Thanks,
-Mahesh.