From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58483) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TbvvX-0003vw-Nl for qemu-devel@nongnu.org; Fri, 23 Nov 2012 11:16:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TbvvW-0007EE-Iv for qemu-devel@nongnu.org; Fri, 23 Nov 2012 11:16:27 -0500 Received: from cantor2.suse.de ([195.135.220.15]:51756 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TbvvW-0007EA-9e for qemu-devel@nongnu.org; Fri, 23 Nov 2012 11:16:26 -0500 Message-ID: <50AFA158.4020502@suse.de> Date: Fri, 23 Nov 2012 17:16:24 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1353686178-27520-1-git-send-email-pbonzini@redhat.com> <1353686178-27520-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1353686178-27520-2-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1.3 1/2] qom: dynamic_cast of NULL is always NULL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aliguori@us.ibm.com, lcapitulino@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com Am 23.11.2012 16:56, schrieb Paolo Bonzini: > Trying to cast a NULL value will cause a crash. Returning > NULL is also sensible, and it is also what the type-unsafe > DO_UPCAST macro does. >=20 > Reported-by: Markus Armbruster > Signed-off-by: Paolo Bonzini I believe we had a lengthy discussion of where to place the NULL checks... seems that was never followed up with a v2 then. In practice however most of NULL assertions stem from misuses of the cast macros, i.e. using them before qdev_create() or so, which we should not hide because they lead to segfaults later. > --- > qom/object.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) >=20 > diff --git a/qom/object.c b/qom/object.c > index d7092b0..2e18c9a 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -417,7 +417,7 @@ void object_delete(Object *obj) > =20 > Object *object_dynamic_cast(Object *obj, const char *typename) > { > - if (object_class_dynamic_cast(object_get_class(obj), typename)) { > + if (obj && object_class_dynamic_cast(object_get_class(obj), typena= me)) { > return obj; > } > =20 This is followed by return NULL; Ack on this part. > @@ -430,7 +430,7 @@ Object *object_dynamic_cast_assert(Object *obj, con= st char *typename) > =20 > inst =3D object_dynamic_cast(obj, typename); > =20 > - if (!inst) { > + if (!inst && obj) { > fprintf(stderr, "Object %p is not an instance of type %s\n", > obj, typename); > abort(); This is followed by return inst; Since this function clearly has assert in the name I don't think this is right. I would expect %p to print 0x0 and the function to abort. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg