From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yuk4A-0001nD-SW for qemu-devel@nongnu.org; Tue, 19 May 2015 12:08:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yuk45-00015n-PO for qemu-devel@nongnu.org; Tue, 19 May 2015 12:08:26 -0400 Received: from cantor2.suse.de ([195.135.220.15]:53436 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yuk45-00014J-Em for qemu-devel@nongnu.org; Tue, 19 May 2015 12:08:21 -0400 Message-ID: <555B5FF3.5090006@suse.de> Date: Tue, 19 May 2015 18:08:19 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1431533649-23115-1-git-send-email-berrange@redhat.com> <1431533649-23115-6-git-send-email-berrange@redhat.com> <555B5C2E.5050903@suse.de> <20150519155528.GF8535@redhat.com> In-Reply-To: <20150519155528.GF8535@redhat.com> Content-Type: text/plain; charset=utf-8 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" Cc: Paolo Bonzini , qemu-devel@nongnu.org Am 19.05.2015 um 17:55 schrieb Daniel P. Berrange: > 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; >> >> This looks fishy. Either we return obj and need to retain a ref for th= at >> (caller's responsibility to unref) or we unref it and return void. >> Or am I misreading the code? >=20 > 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. Heh, that's why I like reviewing my patches myself. That reference can go away via QMP. For the CPU I have a patch pending to fix some corner cases. We can do that as follow-up here though. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Felix Imend=C3=B6rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; = HRB 21284 (AG N=C3=BCrnberg)