On Fri, Jan 08, 2016 at 09:32:02AM +0000, Eric Auger wrote: > This patch aligns the prototype with qemu_fdt_getprop. The caller > can choose whether the function self-asserts on error (passing > &error_fatal as Error ** argument, corresponding to the legacy behavior), > or behaves differently such as simply output a message. > > In this later case the caller can use the new lenp parameter to interpret > the error if any. > > Signed-off-by: Eric Auger <eric.au...@linaro.org> > > --- > > v3 : creation > --- > device_tree.c | 21 ++++++++++++++------- > hw/arm/boot.c | 6 ++++-- > hw/arm/vexpress.c | 6 ++++-- > include/sysemu/device_tree.h | 16 +++++++++++++++- > 4 files changed, 37 insertions(+), 12 deletions(-) > > diff --git a/device_tree.c b/device_tree.c > index 6ecc9da..0e837bf 100644 > --- a/device_tree.c > +++ b/device_tree.c > @@ -338,15 +338,22 @@ const void *qemu_fdt_getprop(void *fdt, const char > *node_path, > } > > uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path, > - const char *property) > + const char *property, int *lenp, Error **errp) > { > int 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); > - exit(1); > + const uint32_t *p; > + > + if (!lenp) { > + lenp = &len; > + } > + p = qemu_fdt_getprop(fdt, node_path, property, lenp, errp); > + if (!p) { > + return 0; > + } else if (*lenp != 4) { > + error_setg(errp, "%s: %s/%s not 4 bytes long (not a cell?)", > + __func__, node_path, property); > + *lenp = -EINVAL; > + return 0; > } > return be32_to_cpu(*p); > } > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index 75f69bf..541b74c 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -386,8 +386,10 @@ static int load_dtb(hwaddr addr, const struct > arm_boot_info *binfo, > return 0; > } > > - acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells"); > - scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells"); > + acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", > + NULL, &error_fatal); > + scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", > + NULL, &error_fatal); > if (acells == 0 || scells == 0) { > fprintf(stderr, "dtb file invalid (#address-cells or #size-cells > 0)\n"); > goto fail; > diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c > index 058abbd..ffe42be 100644 > --- a/hw/arm/vexpress.c > +++ b/hw/arm/vexpress.c > @@ -482,8 +482,10 @@ static void vexpress_modify_dtb(const struct > arm_boot_info *info, void *fdt) > uint32_t acells, scells, intc; > const VEDBoardInfo *daughterboard = (const VEDBoardInfo *)info; > > - acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells"); > - scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells"); > + acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", > + NULL, &error_fatal); > + scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", > + NULL, &error_fatal); > intc = find_int_controller(fdt); > if (!intc) { > /* Not fatal, we just won't provide virtio. This will > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > index 4d7cbb9..5aa750b 100644 > --- a/include/sysemu/device_tree.h > +++ b/include/sysemu/device_tree.h > @@ -60,8 +60,22 @@ int qemu_fdt_setprop_phandle(void *fdt, const char > *node_path, > const void *qemu_fdt_getprop(void *fdt, const char *node_path, > const char *property, int *lenp, > Error **errp); > +/** > + * qemu_fdt_getprop_cell: retrieve the value of a given 4 byte property > + * @fdt: pointer to the device tree blob > + * @node_path: node path > + * @property: name of the property to find > + * @lenp: fdt error if any or -EINVAL if the property size is different from > + * 4 bytes, or 4 (expected length of the property) upon success. > + * @errp: handle to an error object > + * > + * returns the property value on success > + * in case errp is set to &error_fatal, the function auto-asserts > + * on error (legacy behavior)
Avoid re-documenting the semantics of the Error API, as this comment may silently go stale with a patch to the Error API. The function now simply accepts an Error ** and that implies the behaviour of error_fatal. With very few callers there is not much of a legacy to document and it is not a user visible legacy anyway. Otherwise: Reviewed-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com> Regards, Peter > + */ > uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path, > - const char *property); > + const char *property, int *lenp, > + Error **errp); > uint32_t qemu_fdt_get_phandle(void *fdt, const char *path); > uint32_t qemu_fdt_alloc_phandle(void *fdt); > int qemu_fdt_nop_node(void *fdt, const char *node_path); > -- > 1.9.1 >