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

["Andreas Färber" <afaerber@suse.de>]

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.
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

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(-)
> 
> 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)
>  
>  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), typename)) {
>          return obj;
>      }
>  

This is followed by return NULL;

Ack on this part.

> @@ -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.

Regards,
Andreas

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