From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38822) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yujob-0000uo-1J for qemu-devel@nongnu.org; Tue, 19 May 2015 11:52:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YujoW-0001hd-Mi for qemu-devel@nongnu.org; Tue, 19 May 2015 11:52:20 -0400 Received: from cantor2.suse.de ([195.135.220.15]:50252 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YujoW-0001gJ-G9 for qemu-devel@nongnu.org; Tue, 19 May 2015 11:52:16 -0400 Message-ID: <555B5C2E.5050903@suse.de> Date: Tue, 19 May 2015 17:52:14 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1431533649-23115-1-git-send-email-berrange@redhat.com> <1431533649-23115-6-git-send-email-berrange@redhat.com> In-Reply-To: <1431533649-23115-6-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Paolo Bonzini Am 13.05.2015 um 18:14 schrieb Daniel P. Berrange: > +Object *object_new_with_propv(const char *typename, > + Object *parent, > + const char *id, > + Error **errp, > + va_list vargs) > +{ > + Object *obj; > + ObjectClass *klass; > + Error *local_err =3D NULL; > + > + klass =3D object_class_by_name(typename); > + if (!klass) { > + error_setg(errp, "invalid object type: %s", typename); > + return NULL; > + } > + > + if (object_class_is_abstract(klass)) { > + error_setg(errp, "object type '%s' is abstract", typename); > + return NULL; > + } > + obj =3D object_new(typename); > + > + if (object_set_propv(obj, &local_err, vargs) < 0) { > + goto error; > + } > + > + object_property_add_child(parent, id, obj, &local_err); > + if (local_err) { > + goto error; > + } > + > + if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) { > + user_creatable_complete(obj, &local_err); > + if (local_err) { > + object_unparent(obj); > + goto error; > + } > + } > + > + object_unref(OBJECT(obj)); > + return obj; This looks fishy. Either we return obj and need to retain a ref for that (caller's responsibility to unref) or we unref it and return void. Or am I misreading the code? > + > + error: > + if (local_err) { > + error_propagate(errp, local_err); > + } > + object_unref(obj); > + return NULL; > +} [snip] Rest looks good. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)