From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39445) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbX7p-0000pB-9e for qemu-devel@nongnu.org; Mon, 04 Jun 2012 09:15:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SbX7j-00026V-VC for qemu-devel@nongnu.org; Mon, 04 Jun 2012 09:15:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49121) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbX7j-00024L-ND for qemu-devel@nongnu.org; Mon, 04 Jun 2012 09:15:07 -0400 Message-ID: <4FCCB4D2.7070607@redhat.com> Date: Mon, 04 Jun 2012 15:14:58 +0200 From: Igor Mammedov MIME-Version: 1.0 References: <1338394747-17427-1-git-send-email-imammedo@redhat.com> <4FC6533C.3010207@samsung.com> <4FC744FE.3030203@redhat.com> <4FC7533B.2060102@redhat.com> <4FC890CE.8080005@suse.de> <4FC8BDC6.70804@suse.de> In-Reply-To: <4FC8BDC6.70804@suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom-next] qom: make object cast assert if NULL object is passed as argument List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: aliguori@us.ibm.com, Igor Mitsyanko , alexander_barabash@mentor.com, Markus Armbruster , qemu-devel@nongnu.org, Paolo Bonzini On 06/01/2012 03:04 PM, Andreas F=C3=A4rber wrote: > Am 01.06.2012 13:18, schrieb Markus Armbruster: >> Andreas F=C3=A4rber writes: >> >>> Am 31.05.2012 13:17, schrieb Igor Mammedov: >>>> On 05/31/2012 12:16 PM, Paolo Bonzini wrote: >>>>> Il 31/05/2012 10:30, Markus Armbruster ha scritto: >>>>>>>> Makes much sense, but maybe it should be done in OBJECT() cast? = Assert >>>>>>>> when we do OBJECT(NULL). >>>>>> In my opinion, OBJECT(p) where p is a null pointer is perfectly va= lid >>>>>> and should yield a null pointer. >>>>> >>>>> Perhaps object_dynamic_cast and object_dynamic_cast_assert should d= o the >>>>> same? >>>>> >>>> >>>> or better object_dynamic_cast should return NULL if obj is NULL, >>>> after all it's expected that it may return NULL >>> >>> That's what I was suggesting: I think that we should define "NULL is = not >>> of type TYPE_FOO" and thus have the ..._is_... functions return false= , >>> and have the ..._cast_assert assert. >> >> Is it? > > See http://www.mail-archive.com/qemu-devel@nongnu.org/msg113922.html > >> Igor: object_dynamic_cast should return NULL if obj is NULL, >> >> You: have the ..._cast_assert assert [on null argument, I presume] >> >> Doesn't sound like the same suggestion to me :) > > I'll let you to your opinion. :) However, my opinion is that > object_dynamic_cast_assert() should assert (its name should be program)= , > not segfault, and that > object_dynamic_cast()/object_is_type()/type_is_ancestor() should not > assert but return false / NULL. So as to the effects and usability that > pretty much aligns with Igor M., no? If we decide that object_dynamic_cast() should not assert but rather retu= rn NULL the this block in it will be incorrect in to places: if (object_is_type(obj, type_interface)) { assert(!obj->interfaces); <=3D=3D could be replaced with return= NULL obj =3D INTERFACE(obj)->obj; <=3D=3D calls OBJECT_CHECK() -> obj= ect_dynamic_cast_assert () ... [snip] maybe there should be INTERFACE_CHECK and INTERFACE macros calling ..._as= sert and non assert variants respectively? > >> If I understood you correctly: what do such assertions buy us other th= an >> silliness like >> >> p ? some_cast(p) : NULL >> >> ? > > Nack. The point is that currently deployed MY_TYPE(x) should assert > (because nobody expects it to return NULL) and he who does want to > handle NULL can use object_dynamic_cast(p). There's no real change to > what we have except that an error case that was unhandled now is handle= d. > >>> So I still think this patch is correct. It could be accompanied by >>> further patches adding error handling in the remaining functions. >> >> I'm not convinced. > > Shed any light? > > Andreas > --=20 ----- Igor