Re: [Qemu-devel] [PATCH] qom: fix inherited interfaces

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 20 Dec 2013 16:55:44 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The interface type rework had two bugs with interfaces that are inherited
> from a superclass.  First of all, the implementation type name was wrong
> (for example it was subclass::superclass::interface rather than
> just subclass::interface).  Second, interfaces not registering their
> type anymore means that accessing superclass::interface by type name
> will fail.
> 
> This can be fixed by pre-initializing ti->parent_type of subclass::interface
> to superclass::interface.
> 
> Reported-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         Andreas, perhaps this can be squashed with
>         'qom: do not register interface "types" in the type table'.
> 
>  include/qom/object.h |  1 +
>  qom/object.c         | 19 +++++++++++--------
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 5f78847..e0ff212 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -536,6 +536,7 @@ struct InterfaceClass
>      ObjectClass parent_class;
>      /*< private >*/
>      ObjectClass *concrete_class;
> +    Type interface_type;
>  };
>  
>  #define TYPE_INTERFACE "interface"
> diff --git a/qom/object.c b/qom/object.c
> index 750bb62..f696814 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -215,22 +215,25 @@ static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
>  
>  static void type_initialize(TypeImpl *ti);
>  
> -static void type_initialize_interface(TypeImpl *ti, const char *parent)
> +static void type_initialize_interface(TypeImpl *ti, TypeImpl *interface_type,
> +				      TypeImpl *parent_type)
wrong indentation (TABs)

>  {
>      InterfaceClass *new_iface;
>      TypeInfo info = { };
>      TypeImpl *iface_impl;
>  
> -    info.parent = parent;
> -    info.name = g_strdup_printf("%s::%s", ti->name, info.parent);
> +    info.parent = parent_type->name;
> +    info.name = g_strdup_printf("%s::%s", ti->name, interface_type->name);
>      info.abstract = true;
>  
>      iface_impl = type_new(&info);
> +    iface_impl->parent_type = parent_type;
>      type_initialize(iface_impl);
>      g_free((char *)info.name);
>  
>      new_iface = (InterfaceClass *)iface_impl->class;
>      new_iface->concrete_class = ti->class;
> +    new_iface->interface_type = interface_type;
>  
>      ti->class->interfaces = g_slist_append(ti->class->interfaces,
>                                             iface_impl->class);
> @@ -260,8 +263,10 @@ static void type_initialize(TypeImpl *ti)
>          ti->class->interfaces = NULL;
>  
>          for (e = parent->class->interfaces; e; e = e->next) {
> -            ObjectClass *iface = e->data;
> -            type_initialize_interface(ti, object_class_get_name(iface));
> +            InterfaceClass *iface = e->data;
> +            ObjectClass *klass = OBJECT_CLASS(iface);
> +
> +            type_initialize_interface(ti, iface->interface_type, klass->type);
>          }
>  
>          for (i = 0; i < ti->num_interfaces; i++) {
> @@ -278,7 +283,7 @@ static void type_initialize(TypeImpl *ti)
>                  continue;
>              }
>  
> -            type_initialize_interface(ti, ti->interfaces[i].typename);
> +            type_initialize_interface(ti, t, t);
>          }
>      }
>  
> @@ -294,8 +299,6 @@ static void type_initialize(TypeImpl *ti)
>      if (ti->class_init) {
>          ti->class_init(ti->class, ti->class_data);
>      }
> -
> -
>  }
>  
>  static void object_init_with_type(Object *obj, TypeImpl *ti)
> -- 
> 1.8.4.2
> 
>

Tested-By: Igor Mammedov <imammedo@redhat.com>

-- 
Regards,
  Igor