qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4/6] device_tree: qemu_fdt_getprop converted to


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 4/6] device_tree: qemu_fdt_getprop converted to use the error API
Date: Fri, 18 Dec 2015 14:36:06 +0000

On 17 December 2015 at 12:29, Eric Auger <address@hidden> wrote:
> Current qemu_fdt_getprop exits if the property is not found. It is
> sometimes needed to read an optional property, in which case we do
> not wish to exit but simply returns a null value.
>
> This patch converts qemu_fdt_getprop to accept an Error **, and existing
> users are converted to pass &error_fatal. This preserves the existing
> behaviour. Then to use the API with your optional semantic a null
> parameter can be conveyed.
>
> Signed-off-by: Eric Auger <address@hidden>
>
> ---
>
> RFC -> v1:
> - get rid of qemu_fdt_getprop_optional and implement Peter's suggestion
>   that consists in using the error API

This doesn't seem to me like a great way for qemu_fdt_getprop to
report "property not found", because there's no way for the caller
to distinguish "property not found" from "function went wrong
some other way" (since Errors just report human readable strings,
and in any case you're not distinguishing -FDT_ERR_NOTFOUND
from any of the other FDT errors).

If we want to handle "ok if property doesn't exist" then we
could either (a) make the function return NULL on doesn't-exist
and error_report in the other error cases, with the existing
single caller extending its error checking appropriately, or
(b) have the caller that cares about property-may-not-exist
call fdt_getprop() directly.

> Signed-off-by: Eric Auger <address@hidden>
> ---
>  device_tree.c                | 11 ++++++-----
>  include/sysemu/device_tree.h |  3 ++-
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/device_tree.c b/device_tree.c
> index b5d7e0b..c3720c2 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -331,18 +331,18 @@ int qemu_fdt_setprop_string(void *fdt, const char 
> *node_path,
>  }
>
>  const void *qemu_fdt_getprop(void *fdt, const char *node_path,
> -                             const char *property, int *lenp)
> +                             const char *property, int *lenp, Error **errp)
>  {
>      int len;
>      const void *r;
> +
>      if (!lenp) {
>          lenp = &len;
>      }
>      r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);
>      if (!r) {
> -        error_report("%s: Couldn't get %s/%s: %s", __func__,
> -                     node_path, property, fdt_strerror(*lenp));
> -        exit(1);
> +        error_setg(errp, "%s: Couldn't get %s/%s: %s", __func__,
> +                  node_path, property, fdt_strerror(*lenp));
>      }
>      return r;
>  }
> @@ -351,7 +351,8 @@ uint32_t qemu_fdt_getprop_cell(void *fdt, const char 
> *node_path,
>                                 const char *property)
>  {
>      int len;
> -    const uint32_t *p = qemu_fdt_getprop(fdt, node_path, property, &len);
> +    const uint32_t *p = qemu_fdt_getprop(fdt, node_path, property, &len,
> +                                         &error_fatal);
>      if (len != 4) {
>          error_report("%s: %s/%s not 4 bytes long (not a cell?)",
>                       __func__, node_path, property);
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index f9e6e6e..284fd3b 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -33,7 +33,8 @@ int qemu_fdt_setprop_phandle(void *fdt, const char 
> *node_path,
>                               const char *property,
>                               const char *target_node_path);
>  const void *qemu_fdt_getprop(void *fdt, const char *node_path,
> -                             const char *property, int *lenp);
> +                             const char *property, int *lenp,
> +                             Error **errp);

If we change the function it would be nice to add a brief
doc comment while we're touching the prototype in the header.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]