From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46023) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dAEfB-0006WU-E8 for qemu-devel@nongnu.org; Mon, 15 May 2017 07:59:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dAEf8-0002Wa-DJ for qemu-devel@nongnu.org; Mon, 15 May 2017 07:59:45 -0400 Received: from 9.mo69.mail-out.ovh.net ([46.105.56.78]:48703) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dAEf8-0002VW-6e for qemu-devel@nongnu.org; Mon, 15 May 2017 07:59:42 -0400 Received: from player699.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo69.mail-out.ovh.net (Postfix) with ESMTP id 6C2B91FB46 for ; Mon, 15 May 2017 13:59:40 +0200 (CEST) References: <149484833874.20089.4164801378197848306.stgit@bahia.lan> <149484838558.20089.5029926585755792842.stgit@bahia.lan> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: Date: Mon, 15 May 2017 13:59:33 +0200 MIME-Version: 1.0 In-Reply-To: <149484838558.20089.5029926585755792842.stgit@bahia.lan> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/6] spapr: sanitize error handling in spapr_ics_create() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz , qemu-ppc@nongnu.org, qemu-devel@nongnu.org Cc: Bharata B Rao , David Gibson On 05/15/2017 01:39 PM, Greg Kurz wrote: > The spapr_ics_create() function handles errors in a rather convoluted > way, with two local Error * variables. Moreover, failing to parent the > ICS object to the machine should be considered as a bug but it is > currently ignored. I am not sure what should be done for object_property_add_child() errors but QEMU generally uses NULL for 'Error **'. It might be wrong though. As for the local error handling, it is following what is described in qapi/error.h. Isn't it ? Cheers, C. > This patch addresses both issues. > > Signed-off-by: Greg Kurz > --- > hw/ppc/spapr.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 44f7dc7f40e9..c53989bb10b1 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -101,21 +101,26 @@ static ICSState *spapr_ics_create(sPAPRMachineState *spapr, > const char *type_ics, > int nr_irqs, Error **errp) > { > - Error *err = NULL, *local_err = NULL; > + Error *local_err = NULL; > Object *obj; > > obj = object_new(type_ics); > - object_property_add_child(OBJECT(spapr), "ics", obj, NULL); > + object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > object_property_add_const_link(obj, "xics", OBJECT(spapr), &error_abort); > - object_property_set_int(obj, nr_irqs, "nr-irqs", &err); > + object_property_set_int(obj, nr_irqs, "nr-irqs", &local_err); > + if (local_err) { > + goto error; > + } > object_property_set_bool(obj, true, "realized", &local_err); > - error_propagate(&err, local_err); > - if (err) { > - error_propagate(errp, err); > - return NULL; > + if (local_err) { > + goto error; > } > > return ICS_SIMPLE(obj); > + > +error: > + error_propagate(errp, local_err); > + return NULL; > } > > static void xics_system_init(MachineState *machine, int nr_irqs, Error **errp) >