From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yujrq-00035C-Tm for qemu-devel@nongnu.org; Tue, 19 May 2015 11:55:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yujri-0002y4-5o for qemu-devel@nongnu.org; Tue, 19 May 2015 11:55:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59177) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yujrh-0002xv-TK for qemu-devel@nongnu.org; Tue, 19 May 2015 11:55:34 -0400 Date: Tue, 19 May 2015 16:55:29 +0100 From: "Daniel P. Berrange" Message-ID: <20150519155528.GF8535@redhat.com> References: <1431533649-23115-1-git-send-email-berrange@redhat.com> <1431533649-23115-6-git-send-email-berrange@redhat.com> <555B5C2E.5050903@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <555B5C2E.5050903@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= Cc: Paolo Bonzini , qemu-devel@nongnu.org On Tue, May 19, 2015 at 05:52:14PM +0200, Andreas F=C3=A4rber wrote: > 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; >=20 > This looks fishy. Either we return obj and need to retain a ref for tha= t > (caller's responsibility to unref) or we unref it and return void. > Or am I misreading the code? Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done. Regards, Daniel --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|