From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48488) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJ1dg-0006DA-LG for qemu-devel@nongnu.org; Tue, 12 Jan 2016 11:17:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJ1da-000289-Gg for qemu-devel@nongnu.org; Tue, 12 Jan 2016 11:17:44 -0500 Received: from mail-wm0-x232.google.com ([2a00:1450:400c:c09::232]:33141) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJ1da-000282-5b for qemu-devel@nongnu.org; Tue, 12 Jan 2016 11:17:38 -0500 Received: by mail-wm0-x232.google.com with SMTP id f206so260705909wmf.0 for ; Tue, 12 Jan 2016 08:17:37 -0800 (PST) References: <1452245525-3886-1-git-send-email-eric.auger@linaro.org> <1452245525-3886-6-git-send-email-eric.auger@linaro.org> <20160112035840.GF3308@pcrost-box> From: Eric Auger Message-ID: <56952712.8090402@linaro.org> Date: Tue, 12 Jan 2016 17:17:22 +0100 MIME-Version: 1.0 In-Reply-To: <20160112035840.GF3308@pcrost-box> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 5/8] device_tree: qemu_fdt_getprop_cell converted to use the error API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: b.reynal@virtualopensystems.com, peter.maydell@linaro.org, thuth@redhat.com, eric.auger@st.com, patches@linaro.org, qemu-devel@nongnu.org, alex.williamson@redhat.com, qemu-arm@nongnu.org, suravee.suthikulpanit@amd.com, pbonzini@redhat.com, thomas.lendacky@amd.com, alex.bennee@linaro.org, christoffer.dall@linaro.org, david@gibson.dropbear.id.au Hi Peter, On 01/12/2016 04:58 AM, Peter Crosthwaite wrote: > 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 >> >> --- >> >> 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. OK I will remove that comment. Thanks for the reviews and R-b's Best Regards Eric > > Otherwise: > > Reviewed-by: Peter Crosthwaite > > 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 >>