|
From: | Igor Mammedov |
Subject: | Re: [Qemu-devel] [PATCH qom-next] qom: make object cast assert if NULL object is passed as argument |
Date: | Mon, 04 Jun 2012 15:14:58 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 |
On 06/01/2012 03:04 PM, Andreas Färber wrote:
Am 01.06.2012 13:18, schrieb Markus Armbruster:Andreas Färber<address@hidden> 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 valid and should yield a null pointer.Perhaps object_dynamic_cast and object_dynamic_cast_assert should do the same?or better object_dynamic_cast should return NULL if obj is NULL, after all it's expected that it may return NULLThat'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/address@hidden/msg113922.htmlIgor: 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 return NULL the this block in it will be incorrect in to places: if (object_is_type(obj, type_interface)) { assert(!obj->interfaces); <== could be replaced with return NULL obj = INTERFACE(obj)->obj; <== calls OBJECT_CHECK() -> object_dynamic_cast_assert () ... [snip] maybe there should be INTERFACE_CHECK and INTERFACE macros calling ..._assert and non assert variants respectively?
If I understood you correctly: what do such assertions buy us other than 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 handled.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
-- ----- Igor
[Prev in Thread] | Current Thread | [Next in Thread] |