[v5,3/8] device_tree: introduce qemu_fdt_node_path

This new helper routine returns a NULL terminated array of
node paths matching a node name and a compat string.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v4 -> v5:
- support the case where several nodes exist, ie.
  return an array of node paths. Also add Error **
  parameter

v1 -> v2:
- move doc comment in header file
- do not use a fixed size buffer
- break on errors in while loop
- use strcmp instead of strncmp

RFC -> v1:
- improve error handling according to Alex' comments
---
 device_tree.c                | 49 ++++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/device_tree.h | 14 +++++++++++++
 2 files changed, 63 insertions(+)

On 18 January 2016 at 15:16, Eric Auger <eric.auger@linaro.org> wrote:
> This new helper routine returns a NULL terminated array of
> node paths matching a node name and a compat string.
>
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>
> ---
>
> v4 -> v5:
> - support the case where several nodes exist, ie.
>   return an array of node paths. Also add Error **
>   parameter
>
> v1 -> v2:
> - move doc comment in header file
> - do not use a fixed size buffer
> - break on errors in while loop
> - use strcmp instead of strncmp
>
> RFC -> v1:
> - improve error handling according to Alex' comments
> ---
>  device_tree.c                | 49 ++++++++++++++++++++++++++++++++++++++++++++
>  include/sysemu/device_tree.h | 14 +++++++++++++
>  2 files changed, 63 insertions(+)
>
> diff --git a/device_tree.c b/device_tree.c
> index b262c2d..3c88a37 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -231,6 +231,55 @@ static int findnode_nofail(void *fdt, const char *node_path)
>      return offset;
>  }
>
> +char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
> +                          Error **errp)
> +{
> +    int offset, len, ret;
> +    const char *iter_name;
> +    unsigned int path_len = 16, n = 0;
> +    GSList *path_list = NULL, *iter;
> +    char **path_array;
> +
> +    offset = fdt_node_offset_by_compatible(fdt, -1, compat);
> +
> +    while (offset >= 0) {
> +        iter_name = fdt_get_name(fdt, offset, &len);
> +        if (!iter_name) {
> +            offset = len;
> +            break;
> +        }
> +        if (!strcmp(iter_name, name)) {
> +            char *path;
> +
> +            path = g_malloc(path_len);
> +            while ((ret = fdt_get_path(fdt, offset, path, path_len))
> +                  == -FDT_ERR_NOSPACE) {
> +                path_len += 16;
> +                path = g_realloc(path, path_len);
> +            }
> +            path_list = g_slist_prepend(path_list, path);
> +            n++;
> +        }
> +        offset = fdt_node_offset_by_compatible(fdt, offset, compat);
> +    }
> +
> +    if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
> +        error_setg(errp, "%s: abort parsing dt for %s/%s: %s",
> +                   __func__, name, compat, fdt_strerror(offset));

Needs to bail out (freeing memory).

> +    }
> +
> +    path_array = g_new(char *, n + 1);
> +    path_array[n--] = NULL;

0, or '\0'. NULL is a pointer, not the NUL character. (This is a
style issue, not a correctness one.)

> +
> +    for (iter = path_list; iter; iter = iter->next) {
> +        path_array[n--] = iter->data;
> +    }
> +
> +    g_slist_free(path_list);
> +
> +    return path_array;
> +}
> +
>  int qemu_fdt_setprop(void *fdt, const char *node_path,
>                       const char *property, const void *val, int size)
>  {
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index fdf25a4..436b5dd 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -20,6 +20,20 @@ void *load_device_tree(const char *filename_path, int *sizep);
>  void *load_device_tree_from_sysfs(void);
>  #endif
>
> +/**
> + * qemu_fdt_node_path: return the paths of nodes matching a given
> + * name and compat string
> + * @fdt: pointer to the dt blob
> + * @name: node name
> + * @compat: compatibility string
> + * @errp: handle to an error object
> + *
> + * returns a newly allocated NULL-terminated array of node paths.

Should say what we return on error (NULL ?)

> + * Use g_strfreev() to free it.
> + */
> +char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
> +                          Error **errp);
> +
>  int qemu_fdt_setprop(void *fdt, const char *node_path,
>                       const char *property, const void *val, int size);
>  int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
> --
> 1.9.1
>

thanks
-- PMM

Peter,
On 01/25/2016 03:26 PM, Peter Maydell wrote:
> On 18 January 2016 at 15:16, Eric Auger <eric.auger@linaro.org> wrote:
>> This new helper routine returns a NULL terminated array of
>> node paths matching a node name and a compat string.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>
>> ---
>>
>> v4 -> v5:
>> - support the case where several nodes exist, ie.
>>   return an array of node paths. Also add Error **
>>   parameter
>>
>> v1 -> v2:
>> - move doc comment in header file
>> - do not use a fixed size buffer
>> - break on errors in while loop
>> - use strcmp instead of strncmp
>>
>> RFC -> v1:
>> - improve error handling according to Alex' comments
>> ---
>>  device_tree.c                | 49 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/sysemu/device_tree.h | 14 +++++++++++++
>>  2 files changed, 63 insertions(+)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index b262c2d..3c88a37 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -231,6 +231,55 @@ static int findnode_nofail(void *fdt, const char *node_path)
>>      return offset;
>>  }
>>
>> +char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
>> +                          Error **errp)
>> +{
>> +    int offset, len, ret;
>> +    const char *iter_name;
>> +    unsigned int path_len = 16, n = 0;
>> +    GSList *path_list = NULL, *iter;
>> +    char **path_array;
>> +
>> +    offset = fdt_node_offset_by_compatible(fdt, -1, compat);
>> +
>> +    while (offset >= 0) {
>> +        iter_name = fdt_get_name(fdt, offset, &len);
>> +        if (!iter_name) {
>> +            offset = len;
>> +            break;
>> +        }
>> +        if (!strcmp(iter_name, name)) {
>> +            char *path;
>> +
>> +            path = g_malloc(path_len);
>> +            while ((ret = fdt_get_path(fdt, offset, path, path_len))
>> +                  == -FDT_ERR_NOSPACE) {
>> +                path_len += 16;
>> +                path = g_realloc(path, path_len);
>> +            }
>> +            path_list = g_slist_prepend(path_list, path);
>> +            n++;
>> +        }
>> +        offset = fdt_node_offset_by_compatible(fdt, offset, compat);
>> +    }
>> +
>> +    if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
>> +        error_setg(errp, "%s: abort parsing dt for %s/%s: %s",
>> +                   __func__, name, compat, fdt_strerror(offset));
> 
> Needs to bail out (freeing memory).
effectively in case I get an error, it's better to free everything and
return a vector with with a single NULL element, see comment below. I
will correct this.
> 
>> +    }
>> +
>> +    path_array = g_new(char *, n + 1);
>> +    path_array[n--] = NULL;
> 
> 0, or '\0'. NULL is a pointer, not the NUL character. (This is a
> style issue, not a correctness one.)
actually I want to put NULL as the last element of my array and not a
NUL char. The function returns a NULL terminated vector as
g_strsplit_set does, for instance.

Thanks

Eric
> 
>> +
>> +    for (iter = path_list; iter; iter = iter->next) {
>> +        path_array[n--] = iter->data;
>> +    }
>> +
>> +    g_slist_free(path_list);
>> +
>> +    return path_array;
>> +}
>> +
>>  int qemu_fdt_setprop(void *fdt, const char *node_path,
>>                       const char *property, const void *val, int size)
>>  {
>> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
>> index fdf25a4..436b5dd 100644
>> --- a/include/sysemu/device_tree.h
>> +++ b/include/sysemu/device_tree.h
>> @@ -20,6 +20,20 @@ void *load_device_tree(const char *filename_path, int *sizep);
>>  void *load_device_tree_from_sysfs(void);
>>  #endif
>>
>> +/**
>> + * qemu_fdt_node_path: return the paths of nodes matching a given
>> + * name and compat string
>> + * @fdt: pointer to the dt blob
>> + * @name: node name
>> + * @compat: compatibility string
>> + * @errp: handle to an error object
>> + *
>> + * returns a newly allocated NULL-terminated array of node paths.
> 
> Should say what we return on error (NULL ?)
> 
>> + * Use g_strfreev() to free it.
>> + */
>> +char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
>> +                          Error **errp);
>> +
>>  int qemu_fdt_setprop(void *fdt, const char *node_path,
>>                       const char *property, const void *val, int size);
>>  int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
>> --
>> 1.9.1
>>
> 
> thanks
> -- PMM
>