Re: [Qemu-devel] [PATCH 1.3 1/2] qom: dynamic_cast of NULL is always NULL

[Andreas Färber]

Am 23.11.2012 17:25, schrieb Paolo Bonzini:
> Il 23/11/2012 17:16, Andreas Färber ha scritto:
>>>> @@ -430,7 +430,7 @@ Object *object_dynamic_cast_assert(Object *obj, const 
>>>> char *typename)
>>>>  
>>>>      inst = object_dynamic_cast(obj, typename);
>>>>  
>>>> -    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.
> 
> I think it's okay to segfault in this case.
> 
> Otherwise you need to replace this simple cast+check pair:
> 
>     SCSIDevice *s = SCSI_DEVICE(some->long.expressio[n]);
>     if (!s) {
>         return;
>     }
> 
> with something that is more complex:
> 
>     DeviceState *d = some->long.expressio[n];
>     SCSIDevice *s;
> 
>     if (!d) {
>         return;
>     }
>     s = SCSI_DEVICE(d);

Right now our use of those macros guarantees that the result is never
NULL, so there are no such checks! That expectation is broken by your
patch - I'm guessing without reviewing all derived macros.

If having SCSI_BUS() return NULL was your intent you would not have
needed to switch to object_dynamic_cast in your second patch. :)

But let's leave it for Anthony to decide how he wants his macros to work. ;)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg