diff mbox

[v5,3/8] device_tree: introduce qemu_fdt_node_path

Message ID 1453130204-655-4-git-send-email-eric.auger@linaro.org
State New
Headers show

Commit Message

Auger Eric Jan. 18, 2016, 3:16 p.m. UTC
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(+)

-- 
1.9.1

Comments

Peter Maydell Jan. 25, 2016, 2:26 p.m. UTC | #1
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
Auger Eric Jan. 25, 2016, 2:41 p.m. UTC | #2
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

>
diff mbox

Patch

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));
+    }
+
+    path_array = g_new(char *, n + 1);
+    path_array[n--] = NULL;
+
+    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.
+ * 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,