Am 03.04.2012 13:15, schrieb Paolo Bonzini:
> Right now the base Object class has a special NULL type. Change this so
> that we will be able to add class_init and class_base_init callbacks.
> To do this, remove some special casing of ObjectClass that is not really
> necessary.
While the patch itself looks good, we should be aware that this changes
semantics: Before this patch, lack of .parent is identical to .parent =
TYPE_OBJECT; with this patch that would become another base class.
Should be mentioned in the commit message and all TypeInfos need to be
reviewed. I don't think accidentally growing base classes is a good
idea. Maybe whitelist valid base classes in type_register_static() and
abort otherwise? Then at least we'd catch it by just running the
executables.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> include/qemu/object.h | 2 +-
> qom/object.c | 59
> +++++++++++++++++++++++++------------------------
> 2 files changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index ccaea7d..22f646d 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -33,7 +33,7 @@ typedef struct TypeInfo TypeInfo;
> typedef struct InterfaceClass InterfaceClass;
> typedef struct InterfaceInfo InterfaceInfo;
>
> -#define TYPE_OBJECT NULL
> +#define TYPE_OBJECT "object"
>
> /**
> * SECTION:object.h
> diff --git a/qom/object.c b/qom/object.c
> index 6ff1c19..585619d 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -210,7 +210,7 @@ static void type_class_interface_init(TypeImpl *ti,
> InterfaceImpl *iface)
>
> static void type_initialize(TypeImpl *ti)
> {
> - size_t class_size = sizeof(ObjectClass);
> + TypeImpl *parent;
> int i;
>
> if (ti->class) {
> @@ -221,30 +221,24 @@ static void type_initialize(TypeImpl *ti)
> ti->instance_size = type_object_get_size(ti);
>
> ti->class = g_malloc0(ti->class_size);
> - ti->class->type = ti;
> -
> - if (type_has_parent(ti)) {
> - TypeImpl *parent = type_get_parent(ti);
>
> + parent = type_get_parent(ti);
> + if (parent) {
> type_initialize(parent);
>
> - class_size = parent->class_size;
> g_assert(parent->class_size <= ti->class_size);
> + memcpy(ti->class, parent->class, parent->class_size);
> + }
>
> - memcpy((void *)ti->class + sizeof(ObjectClass),
> - (void *)parent->class + sizeof(ObjectClass),
> - parent->class_size - sizeof(ObjectClass));
Is this really safe? Before we would only copy the contents of derived
classes. Missing some explanations IMO.
> + ti->class->type = ti;
>
> - while (parent) {
> - if (parent->class_base_init) {
> - parent->class_base_init(ti->class, ti->class_data);
> - }
> - parent = type_get_parent(parent);
> + while (parent) {
> + if (parent->class_base_init) {
> + parent->class_base_init(ti->class, ti->class_data);
> }
> + parent = type_get_parent(parent);
> }
>
> - memset((void *)ti->class + class_size, 0, ti->class_size - class_size);
> -
> for (i = 0; i < ti->num_interfaces; i++) {
> type_class_interface_init(ti, &ti->interfaces[i]);
> }
> @@ -467,19 +461,6 @@ Object *object_dynamic_cast(Object *obj, const char
> *typename)
> }
>
>
> -static void register_types(void)
> -{
> - static TypeInfo interface_info = {
> - .name = TYPE_INTERFACE,
> - .instance_size = sizeof(Interface),
> - .abstract = true,
> - };
> -
> - type_interface = type_register_static(&interface_info);
> -}
> -
> -type_init(register_types)
> -
> Object *object_dynamic_cast_assert(Object *obj, const char *typename)
> {
> Object *inst;
> @@ -1233,3 +1214,23 @@ void object_property_add_str(Object *obj, const char
> *name,
> property_release_str,
> prop, errp);
> }
> +
> +static void register_types(void)
> +{
> + static TypeInfo interface_info = {
> + .name = TYPE_INTERFACE,
> + .instance_size = sizeof(Interface),
> + .abstract = true,
> + };
> +
> + static TypeInfo object_info = {
> + .name = TYPE_OBJECT,
> + .instance_size = sizeof(Object),
> + .abstract = true,
> + };
> +
> + type_interface = type_register_static(&interface_info);
> + type_register_static(&object_info);
> +}
> +
> +type_init(register_types)
Thanks for moving these down.
Is there a reason not to make them static const just before
register_types()?
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
