[01/27] qom: add the base Object class

This class provides the main building block for QEMU Object Model and is
extensively documented in the header file.  It is largely inspired by GObject.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.objs |    2 +
 hw/object.c   |  469 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/object.h   |  427 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 898 insertions(+), 0 deletions(-)
 create mode 100644 hw/object.c
 create mode 100644 hw/object.h

On 12/20/2011 05:51 PM, Anthony Liguori wrote:
> This class provides the main building block for QEMU Object Model and is
> extensively documented in the header file.  It is largely inspired by GObject.
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> ---
>   Makefile.objs |    2 +
>   hw/object.c   |  469 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/object.h   |  427 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 898 insertions(+), 0 deletions(-)
>   create mode 100644 hw/object.c
>   create mode 100644 hw/object.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index f753d83..b86e8a1 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -122,6 +122,8 @@ common-obj-$(CONFIG_WIN32) += version.o
>
>   common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o ui/spice-display.o spice-qemu-char.o
>
> +common-obj-y += object.o
> +
>   audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
>   audio-obj-$(CONFIG_SDL) += sdlaudio.o
>   audio-obj-$(CONFIG_OSS) += ossaudio.o
> diff --git a/hw/object.c b/hw/object.c
> new file mode 100644
> index 0000000..620e63f
> --- /dev/null
> +++ b/hw/object.c
> @@ -0,0 +1,469 @@
> +/*
> + * QEMU Object Model
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori<aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "object.h"
> +
> +#define MAX_INTERFACES 32
> +
> +typedef struct InterfaceImpl
> +{
> +    const char *parent;
> +    void (*interface_initfn)(ObjectClass *class, void *data);
> +    Type type;
> +} InterfaceImpl;
> +
> +typedef struct TypeImpl
> +{
> +    const char *name;
> +    Type type;

What's the need for "Type"?  You can use simply the TypeImpl * and drop 
type_get_instance.  Outside object.h it can be an opaque pointer.

> +
> +    size_t class_size;
> +
> +    size_t instance_size;
> +
> +    void (*base_init)(ObjectClass *klass);
> +    void (*base_finalize)(ObjectClass *klass);
> +
> +    void (*class_init)(ObjectClass *klass, void *data);
> +    void (*class_finalize)(ObjectClass *klass, void *data);
> +
> +    void *class_data;
> +
> +    void (*instance_init)(Object *obj);
> +    void (*instance_finalize)(Object *obj);
> +
> +    bool abstract;
> +
> +    const char *parent;
> +
> +    ObjectClass *class;
> +
> +    int num_interfaces;
> +    InterfaceImpl interfaces[MAX_INTERFACES];

... this way you can also allocate dynamically and use a variable-sized 
array for interfaces.

> +} TypeImpl;
> +
> +static int num_types = 1;
> +static TypeImpl type_table[1024];

... and you can also drop this.

> +Type type_register_static(const TypeInfo *info)
> +{
> +    Type type = num_types++;
> +    TypeImpl *ti;
> +
> +    ti =&type_table[type];
> +
> +    assert(info->name != NULL);
> +
> +    printf("Added type %s ->  %s\n", info->name, info->parent);
> +
> +    ti->name = info->name;

Why no strdup here?

> +    ti->parent = info->parent;

Please store a Type or a pointer to TypeImpl.  Otherwise, I don't see 
the need to distinguish TypeInfo/InterfaceInfo and 
TypeImpl/InterfaceImpl at all.  The only difference is num_interfaces, and

     for (i = 0; i < ti->num_interfaces; i++) {

can be replaced just as well with

     for (i = 0; i < ti->interfaces[i].type; i++) {

> +    ti->type = type;
> +
> +    ti->class_size = info->class_size;
> +    ti->instance_size = info->instance_size;
> +
> +    ti->base_init = info->base_init;
> +    ti->base_finalize = info->base_finalize;
> +
> +    ti->class_init = info->class_init;
> +    ti->class_finalize = info->class_finalize;
> +    ti->class_data = info->class_data;
> +
> +    ti->instance_init = info->instance_init;
> +    ti->instance_finalize = info->instance_finalize;
> +
> +    ti->abstract = info->abstract;
> +
> +    if (info->interfaces) {
> +        int i;
> +
> +        for (i = 0; info->interfaces[i].type; i++) {
> +            ti->interfaces[i].parent = info->interfaces[i].type;
> +            ti->interfaces[i].interface_initfn = info->interfaces[i].interface_initfn;
> +            ti->num_interfaces++;
> +        }
> +    }
> +
> +    return type;

The return value is unused.  Looks like a premature optimization or a 
leftover from GObject.

> +}
> +
> +static Type type_register_anonymous(const TypeInfo *info)
> +{
> +    Type type = num_types++;
> +    TypeImpl *ti;
> +    char buffer[32];
> +    static int count;
> +
> +    ti =&type_table[type];
> +
> +    snprintf(buffer, sizeof(buffer), "<anonymous-%d>", count++);
> +    ti->name = g_strdup(buffer);

g_strdup_printf, please.  However, this has exactly one use in 
type_class_interface_init.  Can you make the name something like 
<Class::Interface>, so that the meaning is more clear?

> +    ti->parent = g_strdup(info->parent);
> +    ti->type = type;
> +
> +    ti->class_size = info->class_size;
> +    ti->instance_size = info->instance_size;
> +
> +    ti->base_init = info->base_init;
> +    ti->base_finalize = info->base_finalize;
> +
> +    ti->class_init = info->class_init;
> +    ti->class_finalize = info->class_finalize;
> +    ti->class_data = info->class_data;
> +
> +    ti->instance_init = info->instance_init;
> +    ti->instance_finalize = info->instance_finalize;
> +
> +    if (info->interfaces) {
> +        int i;
> +
> +        for (i = 0; info->interfaces[i].type; i++) {
> +            ti->interfaces[i].parent = info->interfaces[i].type;
> +            ti->interfaces[i].interface_initfn = info->interfaces[i].interface_initfn;
> +            ti->num_interfaces++;
> +        }
> +    }
> +
> +    return type;
> +}
> +
> +static TypeImpl *type_get_instance(Type type)
> +{
> +    assert(type != 0);
> +    assert(type<  num_types);
> +
> +    return&type_table[type];
> +}
> +
> +static Type type_get_by_name(const char *name)
> +{
> +    int i;
> +
> +    if (name == NULL) {
> +        return 0;
> +    }
> +
> +    for (i = 1; i<  num_types; i++) {
> +        if (strcmp(name, type_table[i].name) == 0) {
> +            return i;
> +        }
> +    }

Please use a hash table here.  Ultimately object creation might be in 
hot paths.  For example I would like to turn SCSIRequests into QOM 
objects.  It would let me reuse the reference counting as well as help 
with migration.

But in any case, this function should be called as little as possible, 
and should not be static in case other places want to cache the outcome.

> +
> +    return 0;
> +}
> +
> +static void type_class_base_init(TypeImpl *base_ti, const char *typename)
> +{
> +    TypeImpl *ti;
> +
> +    if (!typename) {
> +        return;
> +    }
> +
> +    ti = type_get_instance(type_get_by_name(typename));
> +
> +    type_class_base_init(base_ti, ti->parent);
> +
> +    if (ti->base_init) {
> +        ti->base_init(base_ti->class);
> +    }
> +}
> +
> +static size_t type_class_get_size(TypeImpl *ti)
> +{
> +    if (ti->class_size) {
> +        return ti->class_size;
> +    }
> +
> +    if (ti->parent) {
> +        return type_class_get_size(type_get_instance(type_get_by_name(ti->parent)));
> +    }
> +
> +    return sizeof(ObjectClass);
> +}
> +
> +static void type_class_interface_init(TypeImpl *ti, InterfaceImpl *iface)
> +{
> +    TypeInfo info = {
> +        .instance_size = sizeof(Interface),
> +        .parent = iface->parent,
> +        .class_size = sizeof(InterfaceClass),
> +        .class_init = iface->interface_initfn,
> +        .abstract = true,
> +    };
> +
> +    iface->type = type_register_anonymous(&info);
> +}
> +
> +static void type_class_init(TypeImpl *ti)
> +{
> +    size_t class_size = sizeof(ObjectClass);
> +    int i;
> +
> +    if (ti->class) {
> +        return;
> +    }
> +
> +    ti->class_size = type_class_get_size(ti);
> +
> +    ti->class = g_malloc0(ti->class_size);
> +    ti->class->type = ti->type;
> +
> +    if (ti->parent) {
> +        TypeImpl *ti_parent;
> +
> +        ti_parent = type_get_instance(type_get_by_name(ti->parent));
> +
> +        type_class_init(ti_parent);
> +
> +        class_size = ti_parent->class_size;
> +        assert(ti_parent->class_size<= ti->class_size);
> +
> +        memcpy((void *)ti->class + sizeof(ObjectClass),
> +               (void *)ti_parent->class + sizeof(ObjectClass),
> +               ti_parent->class_size - sizeof(ObjectClass));
> +    }
> +
> +    memset((void *)ti->class + class_size, 0, ti->class_size - class_size);
> +
> +    type_class_base_init(ti, ti->parent);
> +
> +    for (i = 0; i<  ti->num_interfaces; i++) {
> +        type_class_interface_init(ti,&ti->interfaces[i]);
> +    }
> +
> +    if (ti->class_init) {
> +        ti->class_init(ti->class, ti->class_data);
> +    }
> +}
> +
> +static void object_interface_init(Object *obj, InterfaceImpl *iface)
> +{
> +    TypeImpl *ti = type_get_instance(iface->type);
> +    Interface *iface_obj;
> +
> +    iface_obj = INTERFACE(object_new(ti->name));
> +    iface_obj->obj = obj;
> +
> +    obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj);

Please use a QSIMPLEQ.

> +}
> +
> +static void object_init(Object *obj, const char *typename)
> +{
> +    TypeImpl *ti = type_get_instance(type_get_by_name(typename));
> +    int i;
> +
> +    if (ti->parent) {
> +        object_init(obj, ti->parent);
> +    }
> +
> +    for (i = 0; i<  ti->num_interfaces; i++) {
> +        object_interface_init(obj,&ti->interfaces[i]);
> +    }
> +
> +    if (ti->instance_init) {
> +        ti->instance_init(obj);
> +    }
> +}
> +
> +void object_initialize(void *data, const char *typename)
> +{
> +    TypeImpl *ti = type_get_instance(type_get_by_name(typename));

I should be able to pass directly a Type (or a TypeImpl, whatever 
happens of my suggestion above) here.

At the very least provide a function that takes a string and one that 
takes a Type/TypeImpl, and always use the latter when you tail call.


> +    Object *obj = data;
> +
> +    g_assert(ti->instance_size>= sizeof(ObjectClass));

Instead of this, please add to type_class_init an assertion that the 
instance size is bigger than the parent's.

> +
> +    type_class_init(ti);
> +
> +    g_assert(ti->abstract == false);
> +
> +    memset(obj, 0, ti->instance_size);
> +
> +    obj->class = ti->class;
> +
> +    object_init(obj, typename);

What with the double line spacing? :)

> +}
> +
> +static void object_deinit(Object *obj, const char *typename)
> +{
> +    TypeImpl *ti = type_get_instance(type_get_by_name(typename));
> +
> +    if (ti->instance_finalize) {
> +        ti->instance_finalize(obj);
> +    }
> +
> +    while (obj->interfaces) {
> +        Interface *iface_obj = obj->interfaces->data;
> +        obj->interfaces = g_slist_delete_link(obj->interfaces, obj->interfaces);
> +        object_delete(OBJECT(iface_obj));
> +    }
> +
> +    if (ti->parent) {
> +        object_init(obj, ti->parent);
                   ^^^^

Typo, you want deinit.  And again, let's avoid type names.  This is C, 
not BASIC.

> +    }
> +}
> +
> +void object_finalize(void *data)
> +{
> +    Object *obj = data;
> +    TypeImpl *ti = type_get_instance(obj->class->type);
> +
> +    object_deinit(obj, ti->name);
> +}
> +
> +static const char *type_get_name(Type type)
> +{
> +    TypeImpl *ti = type_get_instance(type);
> +    return ti->name;
> +}
> +
> +Object *object_new(const char *typename)
> +{
> +    TypeImpl *ti = type_get_instance(type_get_by_name(typename));
> +    Object *obj;
> +
> +    obj = g_malloc(ti->instance_size);
> +    object_initialize(obj, typename);
> +
> +    return obj;
> +}
> +
> +void object_delete(Object *obj)
> +{
> +    object_finalize(obj);
> +    g_free(obj);
> +}
> +
> +static bool object_is_type(Object *obj, const char *typename)
> +{
> +    Type target_type = type_get_by_name(typename);
> +    Type type = obj->class->type;
> +    GSList *i;
> +
> +    /* Check if typename is a direct ancestor of type */
> +    while (type) {
> +        TypeImpl *ti = type_get_instance(type);
> +
> +        if (ti->type == target_type) {
> +            return true;
> +        }
> +
> +        type = type_get_by_name(ti->parent);
> +    }
> +
> +    /* Check if obj has an interface of typename */
> +    for (i = obj->interfaces; i; i = i->next) {
> +        Interface *iface = i->data;
> +
> +        if (object_is_type(OBJECT(iface), typename)) {
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}
> +
> +Object *object_dynamic_cast(Object *obj, const char *typename)
> +{
> +    GSList *i;
> +
> +    /* Check if typename is a direct ancestor */
> +    if (object_is_type(obj, typename)) {
> +        return obj;
> +    }
> +
> +    /* Check if obj has an interface of typename */
> +    for (i = obj->interfaces; i; i = i->next) {
> +        Interface *iface = i->data;
> +
> +        if (object_is_type(OBJECT(iface), typename)) {
> +            return OBJECT(iface);
> +        }
> +    }
> +
> +    /* Check if obj is an interface and it's containing object is a direct ancestor of typename */
> +    if (object_is_type(obj, TYPE_INTERFACE)) {
> +        Interface *iface = INTERFACE(obj);
> +
> +        if (object_is_type(iface->obj, typename)) {
> +            return iface->obj;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +
> +static void register_interface(void)
> +{
> +    static TypeInfo interface_info = {
> +        .name = TYPE_INTERFACE,
> +        .instance_size = sizeof(Interface),
> +        .abstract = true,
> +    };
> +
> +    type_register_static(&interface_info);
> +}
> +
> +device_init(register_interface);
> +
> +Object *object_dynamic_cast_assert(Object *obj, const char *typename)
> +{
> +    Object *inst;
> +
> +    inst = object_dynamic_cast(obj, typename);
> +
> +    if (!inst) {
> +        fprintf(stderr, "Object %p is not an instance of type %s\n", obj, typename);
> +        abort();
> +    }
> +
> +    return inst;
> +}
> +
> +ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
> +                                              const char *typename)
> +{
> +    Type target_type = type_get_by_name(typename);
> +    Type type = class->type;
> +
> +    while (type) {
> +        TypeImpl *ti = type_get_instance(type);
> +
> +        if (ti->type == target_type) {
> +            return class;
> +        }
> +
> +        type = type_get_by_name(ti->parent);
> +    }
> +
> +    fprintf(stderr, "Object %p is not an instance of type %d\n", class, (int)type);
> +    abort();
> +
> +    return NULL;
> +}
> +
> +const char *object_get_type(Object *obj)
> +{
> +    return type_get_name(obj->class->type);
> +}

Let's not confuse types and type names.

> +ObjectClass *object_get_class(Object *obj)
> +{
> +    return obj->class;
> +}
> +
> +const char *object_class_get_name(ObjectClass *klass)
> +{
> +    return type_get_name(klass->type);
> +}
> diff --git a/hw/object.h b/hw/object.h
> new file mode 100644
> index 0000000..80b7099
> --- /dev/null
> +++ b/hw/object.h
> @@ -0,0 +1,427 @@
> +/*
> + * QEMU Object Model
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori<aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_OBJECT_H
> +#define QEMU_OBJECT_H
> +
> +#include "qemu-common.h"
> +
> +typedef uint64_t Type;

struct TypeImpl;
typedef struct TypeImpl *Type;

> +typedef struct ObjectClass ObjectClass;
> +typedef struct Object Object;
> +
> +typedef struct TypeInfo TypeInfo;
> +
> +typedef struct InterfaceClass InterfaceClass;
> +typedef struct Interface Interface;
> +typedef struct InterfaceInfo InterfaceInfo;
> +
> +#define TYPE_OBJECT NULL
> +
> +/**
> + * SECTION:object.h
> + * @title:Base Object Type System
> + * @short_description: interfaces for creating new types and objects
> + *
> + * The QEMU Object Model provides a framework for registering user creatable
> + * types and instantiating objects from those types.  QOM provides the following
> + * features:
> + *
> + *  - System for dynamically registering types
> + *  - Support for single-inheritance of types
> + *  - Multiple inheritance of stateless interfaces
> + *
> + *<example>
> + *<title>Creating a minimal type</title>
> + *<programlisting>
> + * #include "qdev.h"
> + *
> + * #define TYPE_MY_DEVICE "my-device"

#define TYPE_MY_DEVICE my_device_get_type()

...

static Type my_device_type;

extern void my_device_get_type(void)
{
     return my_device_type();
}

static void my_device_type(void)
{
     my_device_type = type_register_static(&my_device_info);
}

> + * After this initial copy, #TypeInfo::base_init is invoked.  This is meant to
> + * handle the case where a class may have a dynamic field that was copied via
> + * a shallow copy but needs to be deep copied.  #TypeInfo::base_init is called
> + * for* each parent class but not for the class being instantiated.
> + *
> + * Once all of the parent classes have been initialized and their
> + * #TypeInfo::base_init functions have been called, #TypeInfo::class_init is
> + * called to let the class being instantiated provide default initialize for
> + * it's virtual functions.

base_init and base_finalize are unused.  Unless you have a plan for it, 
let's avoid unused features, for now.

> + * # Interfaces #
> + *
> + * Interfaces allow a limited form of multiple inheritance.  Instances are
> + * similar to normal types except for the fact that are only defined by
> + * their classes and never carry any state.  You can cast an object to one
> + * of its #Interface types and vice versa.

You can _dynamically_ cast an object to one of its Interface types and 
vice versa.

> + */
> +
> +/**
> + * ObjectClass:
> + *
> + * The base for all classes.  The only thing that #ObjectClass contains is an
> + * integer type handle.
> + */
> +struct ObjectClass
> +{
> +    /*<  private>*/
> +    Type type;
> +};
> +
> +/**
> + * Object:
> + *
> + * The base for all objects.  The first member of this object is a pointer to
> + * a #ObjectClass.  Since C guarantees that the first member of a structure
> + * always begins at byte 0 of that structure, as long as any sub-object places
> + * its parent as the first member, we can cast directly to a #Object.
> + *
> + * As a result, #Object contains a reference to the objects type as its
> + * first member.  This allows identification of the real type of the object at
> + * run time.
> + *
> + * #Object also contains a list of #Interfaces that this object
> + * implements.
> + */
> +struct Object
> +{
> +    /*<  private>*/
> +    ObjectClass *class;
> +
> +    GSList *interfaces;
> +};
> +
> +/**
> + * TypeInfo:
> + * @name: The name of the type.
> + * @parent: The name of the parent type.
> + * @instance_size: The size of the object (derivative of #Object).  If
> + *   @instance_size is 0, then the size of the object will be the size of the
> + *   parent object.
> + * @instance_init: This function is called to initialize an object.  The parent
> + *   class will have already been initialized so the type is only responsible
> + *   for initializing its own members.
> + * @instance_finalize: This function is called during object destruction.  This
> + *   is called before the parent @instance_finalize function has been called.
> + *   An object should only free the members that are unique to its type in this
> + *   function.
> + * @abstract: If this field is true, then the class is considered abstract and
> + *   cannot be directly instantiated.
> + * @class_size: The size of the class object (derivative of #ObjectClass)
> + *   for this object.  If @class_size is 0, then the size of the class will be
> + *   assumed to be the size of the parent class.  This allows a type to avoid
> + *   implementing an explicit class type if they are not adding additional
> + *   virtual functions.
> + * @base_init: This function is called after memcpy()'ing the base class into
> + *   the new class to reinitialize any members that require deep copy.
> + * @base_finalize: This function is called during a class's destruction and is
> + *   meant to allow any dynamic parameters allocated by @base_init to be
> + *   released.
> + * @class_init: This function is called after all parent class initialization
> + *   has occured to allow a class to set its default virtual method pointers.
> + *   This is also the function to use to override virtual methods from a parent
> + *   class.
> + * @class_finalize: This function is called during class destruction and is
> + *   meant to release and dynamic parameters allocated by @class_init.
> + * @class_data: Data to pass to the @class_init and @class_finalize functions.
> + *   This can be useful when building dynamic classes.
> + * @interfaces: The list of interfaces associated with this type.  This
> + *   should point to a static array that's terminated with a zero filled
> + *   element.
> + */
> +struct TypeInfo
> +{
> +    const char *name;
> +    const char *parent;
> +
> +    size_t instance_size;
> +    void (*instance_init)(Object *obj);
> +    void (*instance_finalize)(Object *obj);
> +
> +    bool abstract;
> +    size_t class_size;
> +
> +    void (*base_init)(ObjectClass *klass);
> +    void (*base_finalize)(ObjectClass *klass);
> +
> +    void (*class_init)(ObjectClass *klass, void *data);
> +    void (*class_finalize)(ObjectClass *klass, void *data);
> +    void *class_data;
> +
> +    InterfaceInfo *interfaces;
> +};
> +
> +/**
> + * OBJECT:
> + * @obj: A derivative of #Object
> + *
> + * Converts an object to a #Object.  Since all objects are #Objects,
> + * this function will always succeed.
> + */
> +#define OBJECT(obj) \
> +    ((Object *)(obj))
> +
> +/**
> + * OBJECT_CHECK:
> + * @type: The C type to use for the return value.
> + * @obj: A derivative of @type to cast.
> + * @name: The QOM typename of @type
> + *
> + * A type safe version of @object_dynamic_cast_assert.  Typically each class
> + * will define a macro based on this type to perform type safe dynamic_casts to
> + * this object type.
> + *
> + * If an invalid object is passed to this function, a run time assert will be
> + * generated.
> + */
> +#define OBJECT_CHECK(type, obj, name) \
> +    ((type *)object_dynamic_cast_assert((Object *)(obj), (name)))
> +
> +/**
> + * OBJECT_CLASS_CHECK:
> + * @class: The C type to use for the return value.
> + * @obj: A derivative of @type to cast.
> + * @name: the QOM typename of @class.
> + *
> + * A type safe version of @object_check_class.  This macro is typically wrapped
> + * by each type to perform type safe casts of a class to a specific class type.
> + */
> +#define OBJECT_CLASS_CHECK(class, obj, name) \
> +    ((class *)object_class_dynamic_cast_assert((ObjectClass *)(obj), (name)))
> +
> +/**
> + * OBJECT_GET_CLASS:
> + * @class: The C type to use for the return value.
> + * @obj: The object to obtain the class for.
> + * @name: The QOM typename of @obj.
> + *
> + * This function will return a specific class for a given object.  Its generally
> + * used by each type to provide a type safe macro to get a specific class type
> + * from an object.
> + */
> +#define OBJECT_GET_CLASS(class, obj, name) \
> +    OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
> +
> +/**
> + * Interface:
> + * @parent: The base class.
> + *
> + * The base for all Interfaces.  This is a subclass of Object.  Subclasses
> + * of #Interface should never have an instance that contains anything other
> + * than a single #Interface member.  Do not attempt to create a type directly
> + * by deriving from #Interface.  Use #TypeInfo::interfaces instead.
> + */
> +struct Interface
> +{
> +    Object parent;
> +
> +    /*<  private>*/
> +
> +    Object *obj;
> +};
> +
> +/**
> + * InterfaceClass:
> + * @parent_class: the base class
> + *
> + * The class for all interfaces.  Subclasses of this class should only add
> + * virtual methods.
> + */
> +struct InterfaceClass
> +{
> +    ObjectClass parent_class;
> +};
> +
> +/**
> + * InterfaceInfo:
> + * @type: The name of the interface.
> + * @interface_initfn: This method is called during class initialization and is
> + *   used to initialize an interface associated with a class.  This function
> + *   should initialize any default virtual functions for a class and/or override
> + *   virtual functions in a parent class.
> + *
> + * The information associated with an interface.
> + */
> +struct InterfaceInfo
> +{
> +    const char *type;
> +
> +    void (*interface_initfn)(ObjectClass *class, void *data);
> +};
> +
> +#define TYPE_INTERFACE "interface"

Let's make TYPE_* constants pointers, not strings.

> +
> +/**
> + * INTERFACE:
> + * @obj: the object to cast to an Interface
> + *
> + * Type safe macro to cast to #Interface
> + */
> +#define INTERFACE(obj) OBJECT_CHECK(Interface, obj, TYPE_INTERFACE)
> +
> +/**
> + * object_new:
> + * @typename: The name of the type of the object to instantiate.
> + *
> + * This function will initialize a new object using heap allocated memory.  This
> + * function should be paired with object_delete() to free the resources
> + * associated with the object.
> + *
> + * Returns: The newly allocated and instantiated object.
> + */
> +Object *object_new(const char *typename);
> +
> +/**
> + * object_delete:
> + * @obj: The object to free.
> + *
> + * Finalize an object and then free the memory associated with it.  This should
> + * be paired with object_new() to free the resources associated with an object.
> + */
> +void object_delete(Object *obj);
> +
> +/**
> + * object_initialize:
> + * @obj: A pointer to the memory to be used for the object.
> + * @typename: The name of the type of the object to instantiate.
> + *
> + * This function will initialize an object.  The memory for the object should
> + * have already been allocated.
> + */
> +void object_initialize(void *obj, const char *typename);
> +
> +/**
> + * object_finalize:
> + * @obj: The object to finalize.
> + *
> + * This function destroys and object without freeing the memory associated with
> + * it.
> + */
> +void object_finalize(void *obj);
> +
> +/**
> + * object_dynamic_cast:
> + * @obj: The object to cast.
> + * @typename: The @typename to cast to.
> + *
> + * This function will determine if @obj is-a @typename.  @obj can refer to an
> + * object or an interface associated with an object.
> + *
> + * Returns: This function returns @obj on success or #NULL on failure.
> + */
> +Object *object_dynamic_cast(Object *obj, const char *typename);
> +
> +/**
> + * @object_dynamic_cast_assert:
> + *
> + * See object_dynamic_cast() for a description of the parameters of this
> + * function.  The only difference in behavior is that this function asserts
> + * instead of returning #NULL on failure.
> + */
> +Object *object_dynamic_cast_assert(Object *obj, const char *typename);
> +
> +/**
> + * object_get_class:
> + * @obj: A derivative of #Object
> + *
> + * Returns: The #ObjectClass of the type associated with @obj.
> + */
> +ObjectClass *object_get_class(Object *obj);
> +
> +/**
> + * object_get_type:
> + * @obj: A derivative of #Object.
> + *
> + * Returns: The QOM typename of @obj.
> + */
> +const char *object_get_type(Object *obj);
> +
> +/**
> + * type_register_static:
> + * @info: The #TypeInfo of the new type
> + *
> + * Returns: 0 on failure, the new #Type on success.
> + */
> +Type type_register_static(const TypeInfo *info);
> +
> +/**
> + * object_class_dynamic_cast_assert:
> + * @klass: The #ObjectClass to attempt to cast.
> + * @typename: The QOM typename of the class to cast to.
> + *
> + * Returns: This function always returns @klass and asserts on failure.
> + */
> +ObjectClass *object_class_dynamic_cast_assert(ObjectClass *klass,
> +                                              const char *typename);
> +
> +/**
> + * object_class_get_name:
> + * @klass: The class to obtain the QOM typename for.
> + *
> + * Returns: The QOM typename for @klass.
> + */
> +const char *object_class_get_name(ObjectClass *klass);
> +
> +#endif

On 12/21/2011 07:35 AM, Paolo Bonzini wrote:
> On 12/20/2011 05:51 PM, Anthony Liguori wrote:
>> This class provides the main building block for QEMU Object Model and is
>> extensively documented in the header file. It is largely inspired by GObject.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>> Makefile.objs | 2 +
>> hw/object.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> hw/object.h | 427 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 898 insertions(+), 0 deletions(-)
>> create mode 100644 hw/object.c
>> create mode 100644 hw/object.h
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index f753d83..b86e8a1 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -122,6 +122,8 @@ common-obj-$(CONFIG_WIN32) += version.o
>>
>> common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o
>> ui/spice-display.o spice-qemu-char.o
>>
>> +common-obj-y += object.o
>> +
>> audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
>> audio-obj-$(CONFIG_SDL) += sdlaudio.o
>> audio-obj-$(CONFIG_OSS) += ossaudio.o
>> diff --git a/hw/object.c b/hw/object.c
>> new file mode 100644
>> index 0000000..620e63f
>> --- /dev/null
>> +++ b/hw/object.c
>> @@ -0,0 +1,469 @@
>> +/*
>> + * QEMU Object Model
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + * Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "object.h"
>> +
>> +#define MAX_INTERFACES 32
>> +
>> +typedef struct InterfaceImpl
>> +{
>> + const char *parent;
>> + void (*interface_initfn)(ObjectClass *class, void *data);
>> + Type type;
>> +} InterfaceImpl;
>> +
>> +typedef struct TypeImpl
>> +{
>> + const char *name;
>> + Type type;
>
> What's the need for "Type"? You can use simply the TypeImpl * and drop
> type_get_instance. Outside object.h it can be an opaque pointer.

It's a bit nicer for type_register to return a handle that can later be 
unregistered (although that's not currently implemented).

You could have it return TypeImpl * of course.  GObject uses a simpler GType but 
they don't have the notion of a symbolic type name.

I used a symbolic type name to avoid the problem of dependencies.  In order to 
create a type in gobject, you have to reference the parent's GType which usually 
means you have to call the _get_type() function which acts as a singleton which 
registers the type.

Since you have to specify the parent via a function call, you can't define the 
type in a unit-level static structure which I viewed as a critical requirement.

So it might be worth refactoring it away but I'd prefer to do that in the future 
once we're sure it's not useful.

>> +
>> + size_t class_size;
>> +
>> + size_t instance_size;
>> +
>> + void (*base_init)(ObjectClass *klass);
>> + void (*base_finalize)(ObjectClass *klass);
>> +
>> + void (*class_init)(ObjectClass *klass, void *data);
>> + void (*class_finalize)(ObjectClass *klass, void *data);
>> +
>> + void *class_data;
>> +
>> + void (*instance_init)(Object *obj);
>> + void (*instance_finalize)(Object *obj);
>> +
>> + bool abstract;
>> +
>> + const char *parent;
>> +
>> + ObjectClass *class;
>> +
>> + int num_interfaces;
>> + InterfaceImpl interfaces[MAX_INTERFACES];
>
> ... this way you can also allocate dynamically and use a variable-sized array
> for interfaces.

This could be made dynamic fairly easily.


>> +} TypeImpl;
>> +
>> +static int num_types = 1;
>> +static TypeImpl type_table[1024];
>
> ... and you can also drop this.

Yes, this could be gotten rid of.

>
>> +Type type_register_static(const TypeInfo *info)
>> +{
>> + Type type = num_types++;
>> + TypeImpl *ti;
>> +
>> + ti =&type_table[type];
>> +
>> + assert(info->name != NULL);
>> +
>> + printf("Added type %s -> %s\n", info->name, info->parent);
>> +
>> + ti->name = info->name;
>
> Why no strdup here?

'static' means that the strings are const char *.

>
>> + ti->parent = info->parent;
>
> Please store a Type or a pointer to TypeImpl.

It needs to be a symbolic name because the TypeImpl may not exist yet since we 
can't guarantee registration order.

This ends up being a tremendous simplification because we don't have to care 
about type registration order.

> Otherwise, I don't see the need to
> distinguish TypeInfo/InterfaceInfo and TypeImpl/InterfaceImpl at all. The only
> difference is num_interfaces, and
>
> for (i = 0; i < ti->num_interfaces; i++) {
>
> can be replaced just as well with
>
> for (i = 0; i < ti->interfaces[i].type; i++) {

InterfaceInfo is specifically limited in what it contains to prevent someone 
from shooting themselves in the foot.  It may be possible to just use TypeImpl 
in the backend but we should keep InterfaceInfo limited.

>> + ti->type = type;
>> +
>> + ti->class_size = info->class_size;
>> + ti->instance_size = info->instance_size;
>> +
>> + ti->base_init = info->base_init;
>> + ti->base_finalize = info->base_finalize;
>> +
>> + ti->class_init = info->class_init;
>> + ti->class_finalize = info->class_finalize;
>> + ti->class_data = info->class_data;
>> +
>> + ti->instance_init = info->instance_init;
>> + ti->instance_finalize = info->instance_finalize;
>> +
>> + ti->abstract = info->abstract;
>> +
>> + if (info->interfaces) {
>> + int i;
>> +
>> + for (i = 0; info->interfaces[i].type; i++) {
>> + ti->interfaces[i].parent = info->interfaces[i].type;
>> + ti->interfaces[i].interface_initfn = info->interfaces[i].interface_initfn;
>> + ti->num_interfaces++;
>> + }
>> + }
>> +
>> + return type;
>
> The return value is unused. Looks like a premature optimization or a leftover
> from GObject.

It will be used to allow type unregistration.  Think about dynamic 
loadable/unloadable modules.

>
>> +}
>> +
>> +static Type type_register_anonymous(const TypeInfo *info)
>> +{
>> + Type type = num_types++;
>> + TypeImpl *ti;
>> + char buffer[32];
>> + static int count;
>> +
>> + ti =&type_table[type];
>> +
>> + snprintf(buffer, sizeof(buffer), "<anonymous-%d>", count++);
>> + ti->name = g_strdup(buffer);
>
> g_strdup_printf, please. However, this has exactly one use in
> type_class_interface_init. Can you make the name something like
> <Class::Interface>, so that the meaning is more clear?

Ack.

>> +static Type type_get_by_name(const char *name)
>> +{
>> + int i;
>> +
>> + if (name == NULL) {
>> + return 0;
>> + }
>> +
>> + for (i = 1; i< num_types; i++) {
>> + if (strcmp(name, type_table[i].name) == 0) {
>> + return i;
>> + }
>> + }
>
> Please use a hash table here. Ultimately object creation might be in hot paths.
> For example I would like to turn SCSIRequests into QOM objects. It would let me
> reuse the reference counting as well as help with migration.

Ack.

>
> But in any case, this function should be called as little as possible, and
> should not be static in case other places want to cache the outcome.

Hrm, I don't think it should ever be used outside of object.c.  What are you 
thinking re: caching.

>> +static void object_interface_init(Object *obj, InterfaceImpl *iface)
>> +{
>> + TypeImpl *ti = type_get_instance(iface->type);
>> + Interface *iface_obj;
>> +
>> + iface_obj = INTERFACE(object_new(ti->name));
>> + iface_obj->obj = obj;
>> +
>> + obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj);
>
> Please use a QSIMPLEQ.

I saw your github comment and looked at it.  Turns out, Interface shouldn't be 
public at all (because you only ever subclass InterfaceClass, never Interface). 
  You'll see in the header that I say something like, "don't ever subclass this" 
but why even expose something that a user isn't supposed to use?

So locally, I've moved Interface to be declared in object.c which means that in 
order to use QSIMPLEQ, I would have to make it public again since Object needs 
to hold a list of Interfaces.

Using GSList has the nice property of keeping the contents of the list totally 
private.

>> +}
>> +
>> +static void object_init(Object *obj, const char *typename)
>> +{
>> + TypeImpl *ti = type_get_instance(type_get_by_name(typename));
>> + int i;
>> +
>> + if (ti->parent) {
>> + object_init(obj, ti->parent);
>> + }
>> +
>> + for (i = 0; i< ti->num_interfaces; i++) {
>> + object_interface_init(obj,&ti->interfaces[i]);
>> + }
>> +
>> + if (ti->instance_init) {
>> + ti->instance_init(obj);
>> + }
>> +}
>> +
>> +void object_initialize(void *data, const char *typename)
>> +{
>> + TypeImpl *ti = type_get_instance(type_get_by_name(typename));
>
> I should be able to pass directly a Type (or a TypeImpl, whatever happens of my
> suggestion above) here.

The Type is not public.  It's a dynamic value and unless we introduce a 
_get_type() function (which would create a dependency problem), there's no easy 
way to do it.

Symbolic names really help simplify things by allowing dynamic dependency 
resolution.

> At the very least provide a function that takes a string and one that takes a
> Type/TypeImpl, and always use the latter when you tail call.

You're concerned about performance?

>
>> + Object *obj = data;
>> +
>> + g_assert(ti->instance_size>= sizeof(ObjectClass));
>
> Instead of this, please add to type_class_init an assertion that the instance
> size is bigger than the parent's.

Will take a look.

>
>> +
>> + type_class_init(ti);
>> +
>> + g_assert(ti->abstract == false);
>> +
>> + memset(obj, 0, ti->instance_size);
>> +
>> + obj->class = ti->class;
>> +
>> + object_init(obj, typename);
>
> What with the double line spacing? :)

I have no clue :-)  Will fix.

>> +}
>> +
>> +static void object_deinit(Object *obj, const char *typename)
>> +{
>> + TypeImpl *ti = type_get_instance(type_get_by_name(typename));
>> +
>> + if (ti->instance_finalize) {
>> + ti->instance_finalize(obj);
>> + }
>> +
>> + while (obj->interfaces) {
>> + Interface *iface_obj = obj->interfaces->data;
>> + obj->interfaces = g_slist_delete_link(obj->interfaces, obj->interfaces);
>> + object_delete(OBJECT(iface_obj));
>> + }
>> +
>> + if (ti->parent) {
>> + object_init(obj, ti->parent);
> ^^^^
>
> Typo, you want deinit. And again, let's avoid type names. This is C, not BASIC.

Ack on the deinit.  But see above re: type names.

>> +const char *object_get_type(Object *obj)
>> +{
>> + return type_get_name(obj->class->type);
>> +}
>
> Let's not confuse types and type names.

Ack.

>> +ObjectClass *object_get_class(Object *obj)
>> +{
>> + return obj->class;
>> +}
>> +
>> +const char *object_class_get_name(ObjectClass *klass)
>> +{
>> + return type_get_name(klass->type);
>> +}
>> diff --git a/hw/object.h b/hw/object.h
>> new file mode 100644
>> index 0000000..80b7099
>> --- /dev/null
>> +++ b/hw/object.h
>> @@ -0,0 +1,427 @@
>> +/*
>> + * QEMU Object Model
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + * Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef QEMU_OBJECT_H
>> +#define QEMU_OBJECT_H
>> +
>> +#include "qemu-common.h"
>> +
>> +typedef uint64_t Type;
>
> struct TypeImpl;
> typedef struct TypeImpl *Type;

Not so sure on this.

>
>> +typedef struct ObjectClass ObjectClass;
>> +typedef struct Object Object;
>> +
>> +typedef struct TypeInfo TypeInfo;
>> +
>> +typedef struct InterfaceClass InterfaceClass;
>> +typedef struct Interface Interface;
>> +typedef struct InterfaceInfo InterfaceInfo;
>> +
>> +#define TYPE_OBJECT NULL
>> +
>> +/**
>> + * SECTION:object.h
>> + * @title:Base Object Type System
>> + * @short_description: interfaces for creating new types and objects
>> + *
>> + * The QEMU Object Model provides a framework for registering user creatable
>> + * types and instantiating objects from those types. QOM provides the following
>> + * features:
>> + *
>> + * - System for dynamically registering types
>> + * - Support for single-inheritance of types
>> + * - Multiple inheritance of stateless interfaces
>> + *
>> + *<example>
>> + *<title>Creating a minimal type</title>
>> + *<programlisting>
>> + * #include "qdev.h"
>> + *
>> + * #define TYPE_MY_DEVICE "my-device"
>
> #define TYPE_MY_DEVICE my_device_get_type()

This makes things pretty ugly.

> ...
>
> static Type my_device_type;
>
> extern void my_device_get_type(void)
> {
> return my_device_type();
> }
>
> static void my_device_type(void)
> {
> my_device_type = type_register_static(&my_device_info);

It cannot be reference an external structure.  You would have to do:

Type my_device_get_type(void)
{
     static Type my_device_type;

     if (my_device_type == 0) {
         TypeInfo my_device_info = {
             .name = "my-device",
             .parent = parent_device_get_type(),
         };

         my_device_type = type_register_static(&my_device_info);
     }

     return my_device_type;
}

static void my_device_module_init(void)
{
     my_device_get_type(); // dummy call to register type
}

device_init(my_device_module_init);

Besides looking uglier, it means that types don't exist until you fetch all of 
the types.  That's why you still need a module init.  The nasty bit is that if 
you do the module init, then you need to make sure that you load dynamic modules 
in dependency order first.

One really nice thing about having a string-lookup method to resolve type names 
is that it makes it very easy to support on-demand module loading.  All we need 
to do is look for a module if a type lookup fails and load it.

> }
>
>> + * After this initial copy, #TypeInfo::base_init is invoked. This is meant to
>> + * handle the case where a class may have a dynamic field that was copied via
>> + * a shallow copy but needs to be deep copied. #TypeInfo::base_init is called
>> + * for* each parent class but not for the class being instantiated.
>> + *
>> + * Once all of the parent classes have been initialized and their
>> + * #TypeInfo::base_init functions have been called, #TypeInfo::class_init is
>> + * called to let the class being instantiated provide default initialize for
>> + * it's virtual functions.
>
> base_init and base_finalize are unused. Unless you have a plan for it, let's
> avoid unused features, for now.

Tentative Ack unless I can come up with a compelling counter argument ;-)

>> + * # Interfaces #
>> + *
>> + * Interfaces allow a limited form of multiple inheritance. Instances are
>> + * similar to normal types except for the fact that are only defined by
>> + * their classes and never carry any state. You can cast an object to one
>> + * of its #Interface types and vice versa.
>
> You can _dynamically_ cast an object to one of its Interface types and vice versa.

Ack.

>> + */
>> +
>> +/**
>> + * ObjectClass:
>> + *
>> + * The base for all classes. The only thing that #ObjectClass contains is an
>> + * integer type handle.
>> + */
>> +struct ObjectClass
>> +{
>> + /*< private>*/
>> + Type type;
>> +};
>> +
>> +/**
>> + * Object:
>> + *
>> + * The base for all objects. The first member of this object is a pointer to
>> + * a #ObjectClass. Since C guarantees that the first member of a structure
>> + * always begins at byte 0 of that structure, as long as any sub-object places
>> + * its parent as the first member, we can cast directly to a #Object.
>> + *
>> + * As a result, #Object contains a reference to the objects type as its
>> + * first member. This allows identification of the real type of the object at
>> + * run time.
>> + *
>> + * #Object also contains a list of #Interfaces that this object
>> + * implements.
>> + */
>> +struct Object
>> +{
>> + /*< private>*/
>> + ObjectClass *class;
>> +
>> + GSList *interfaces;
>> +};
>> +
>> +/**
>> + * TypeInfo:
>> + * @name: The name of the type.
>> + * @parent: The name of the parent type.
>> + * @instance_size: The size of the object (derivative of #Object). If
>> + * @instance_size is 0, then the size of the object will be the size of the
>> + * parent object.
>> + * @instance_init: This function is called to initialize an object. The parent
>> + * class will have already been initialized so the type is only responsible
>> + * for initializing its own members.
>> + * @instance_finalize: This function is called during object destruction. This
>> + * is called before the parent @instance_finalize function has been called.
>> + * An object should only free the members that are unique to its type in this
>> + * function.
>> + * @abstract: If this field is true, then the class is considered abstract and
>> + * cannot be directly instantiated.
>> + * @class_size: The size of the class object (derivative of #ObjectClass)
>> + * for this object. If @class_size is 0, then the size of the class will be
>> + * assumed to be the size of the parent class. This allows a type to avoid
>> + * implementing an explicit class type if they are not adding additional
>> + * virtual functions.
>> + * @base_init: This function is called after memcpy()'ing the base class into
>> + * the new class to reinitialize any members that require deep copy.
>> + * @base_finalize: This function is called during a class's destruction and is
>> + * meant to allow any dynamic parameters allocated by @base_init to be
>> + * released.
>> + * @class_init: This function is called after all parent class initialization
>> + * has occured to allow a class to set its default virtual method pointers.
>> + * This is also the function to use to override virtual methods from a parent
>> + * class.
>> + * @class_finalize: This function is called during class destruction and is
>> + * meant to release and dynamic parameters allocated by @class_init.
>> + * @class_data: Data to pass to the @class_init and @class_finalize functions.
>> + * This can be useful when building dynamic classes.
>> + * @interfaces: The list of interfaces associated with this type. This
>> + * should point to a static array that's terminated with a zero filled
>> + * element.
>> + */
>> +struct TypeInfo
>> +{
>> + const char *name;
>> + const char *parent;
>> +
>> + size_t instance_size;
>> + void (*instance_init)(Object *obj);
>> + void (*instance_finalize)(Object *obj);
>> +
>> + bool abstract;
>> + size_t class_size;
>> +
>> + void (*base_init)(ObjectClass *klass);
>> + void (*base_finalize)(ObjectClass *klass);
>> +
>> + void (*class_init)(ObjectClass *klass, void *data);
>> + void (*class_finalize)(ObjectClass *klass, void *data);
>> + void *class_data;
>> +
>> + InterfaceInfo *interfaces;
>> +};
>> +
>> +/**
>> + * OBJECT:
>> + * @obj: A derivative of #Object
>> + *
>> + * Converts an object to a #Object. Since all objects are #Objects,
>> + * this function will always succeed.
>> + */
>> +#define OBJECT(obj) \
>> + ((Object *)(obj))
>> +
>> +/**
>> + * OBJECT_CHECK:
>> + * @type: The C type to use for the return value.
>> + * @obj: A derivative of @type to cast.
>> + * @name: The QOM typename of @type
>> + *
>> + * A type safe version of @object_dynamic_cast_assert. Typically each class
>> + * will define a macro based on this type to perform type safe dynamic_casts to
>> + * this object type.
>> + *
>> + * If an invalid object is passed to this function, a run time assert will be
>> + * generated.
>> + */
>> +#define OBJECT_CHECK(type, obj, name) \
>> + ((type *)object_dynamic_cast_assert((Object *)(obj), (name)))
>> +
>> +/**
>> + * OBJECT_CLASS_CHECK:
>> + * @class: The C type to use for the return value.
>> + * @obj: A derivative of @type to cast.
>> + * @name: the QOM typename of @class.
>> + *
>> + * A type safe version of @object_check_class. This macro is typically wrapped
>> + * by each type to perform type safe casts of a class to a specific class type.
>> + */
>> +#define OBJECT_CLASS_CHECK(class, obj, name) \
>> + ((class *)object_class_dynamic_cast_assert((ObjectClass *)(obj), (name)))
>> +
>> +/**
>> + * OBJECT_GET_CLASS:
>> + * @class: The C type to use for the return value.
>> + * @obj: The object to obtain the class for.
>> + * @name: The QOM typename of @obj.
>> + *
>> + * This function will return a specific class for a given object. Its generally
>> + * used by each type to provide a type safe macro to get a specific class type
>> + * from an object.
>> + */
>> +#define OBJECT_GET_CLASS(class, obj, name) \
>> + OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>> +
>> +/**
>> + * Interface:
>> + * @parent: The base class.
>> + *
>> + * The base for all Interfaces. This is a subclass of Object. Subclasses
>> + * of #Interface should never have an instance that contains anything other
>> + * than a single #Interface member. Do not attempt to create a type directly
>> + * by deriving from #Interface. Use #TypeInfo::interfaces instead.
>> + */
>> +struct Interface
>> +{
>> + Object parent;
>> +
>> + /*< private>*/
>> +
>> + Object *obj;
>> +};
>> +
>> +/**
>> + * InterfaceClass:
>> + * @parent_class: the base class
>> + *
>> + * The class for all interfaces. Subclasses of this class should only add
>> + * virtual methods.
>> + */
>> +struct InterfaceClass
>> +{
>> + ObjectClass parent_class;
>> +};
>> +
>> +/**
>> + * InterfaceInfo:
>> + * @type: The name of the interface.
>> + * @interface_initfn: This method is called during class initialization and is
>> + * used to initialize an interface associated with a class. This function
>> + * should initialize any default virtual functions for a class and/or override
>> + * virtual functions in a parent class.
>> + *
>> + * The information associated with an interface.
>> + */
>> +struct InterfaceInfo
>> +{
>> + const char *type;
>> +
>> + void (*interface_initfn)(ObjectClass *class, void *data);
>> +};
>> +
>> +#define TYPE_INTERFACE "interface"
>
> Let's make TYPE_* constants pointers, not strings.

See above.

Thanks for the thorough review!

Regards,

Anthony Liguori

On 12/21/2011 03:35 PM, Anthony Liguori wrote:
>>> +Type type_register_static(const TypeInfo *info)
>>> +{
>>> + Type type = num_types++;
>>> + TypeImpl *ti;
>>> +
>>> + ti =&type_table[type];
>>> +
>>> + assert(info->name != NULL);
>>> +
>>> + printf("Added type %s -> %s\n", info->name, info->parent);
>>> +
>>> + ti->name = info->name;
>>
>> Why no strdup here?
>
> 'static' means that the strings are const char *.

Sure, but if you later want to unregister, the ownership of ->name and 
->parent need to be the same for all paths.  In this case 
type_register_anonymous makes it "owned by object.c" and 
type_register_static makes it "owned by caller".

>>> + ti->parent = info->parent;
>>
>> Please store a Type or a pointer to TypeImpl.
>
> It needs to be a symbolic name because the TypeImpl may not exist yet
> since we can't guarantee registration order.
>
> This ends up being a tremendous simplification because we don't have to
> care about type registration order.

Got it now.  However, let's cache the Type as soon as we resolve it the 
first time.

>> Otherwise, I don't see the need to
>> distinguish TypeInfo/InterfaceInfo and TypeImpl/InterfaceImpl at all.
>> The only
>> difference is num_interfaces, and
>>
>> for (i = 0; i < ti->num_interfaces; i++) {
>>
>> can be replaced just as well with
>>
>> for (i = 0; i < ti->interfaces[i].type; i++) {
>
> InterfaceInfo is specifically limited in what it contains to prevent
> someone from shooting themselves in the foot. It may be possible to just
> use TypeImpl in the backend but we should keep InterfaceInfo limited.

Let's keep Impl/Info separate, but use the difference so that we can 
cache name->type mappings as much as possible.

>>> + return type;
>>
>> The return value is unused. Looks like a premature optimization or a
>> leftover
>> from GObject.
>
> It will be used to allow type unregistration. Think about dynamic
> loadable/unloadable modules.

Ack.

>>> + for (i = 1; i< num_types; i++) {
>>> + if (strcmp(name, type_table[i].name) == 0) {
>>> + return i;
>>> + }
>>> + }
>>
>> Please use a hash table here. Ultimately object creation might be
>> in hot paths. For example I would like to turn SCSIRequests into
>> QOM objects. It would let me reuse the reference counting as well
>> as help with migration.
>
> Ack.
>
>> But in any case, this function should be called as little as
>> possible, and should not be static in case other places want to
>> cache the outcome.
>
> Hrm, I don't think it should ever be used outside of object.c. What are
> you thinking re: caching.

I'm thinking of using

>>> +static void object_interface_init(Object *obj, InterfaceImpl *iface)
>>> +{
>>> + TypeImpl *ti = type_get_instance(iface->type);
>>> + Interface *iface_obj;
>>> +
>>> + iface_obj = INTERFACE(object_new(ti->name));
>>> + iface_obj->obj = obj;
>>> +
>>> + obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj);
>>
>> Please use a QSIMPLEQ.
>
> I saw your github comment and looked at it. Turns out, Interface
> shouldn't be public at all (because you only ever subclass
> InterfaceClass, never Interface).

I agree that Interface and INTERFACE() should be private to object.c, 
but it's not a big deal to leave them as opaque in object.h.  Leaving 
Interface as an opaque type is enough in order to use QSIMPLEQ.

>>> +void object_initialize(void *data, const char *typename)
>>> +{
>>> + TypeImpl *ti = type_get_instance(type_get_by_name(typename));
>>
>> I should be able to pass directly a Type (or a TypeImpl, whatever
>> happens of my
>> suggestion above) here.
>
> The Type is not public. It's a dynamic value and unless we introduce a
> _get_type() function (which would create a dependency problem), there's
> no easy way to do it.

True, we need to keep types as strings, but we can do some kind of lazy 
initialization.  See IRC conversation + later.

> Symbolic names really help simplify things by allowing dynamic
> dependency resolution.

Yes, I understand this now.  But once we're out of the initialization 
path we should be able to avoid them in hot paths.
>> At the very least provide a function that takes a string and one that
>> takes a Type/TypeImpl, and always use the latter when you tail call.
>
> You're concerned about performance?

Yes.  At least for a simple subclass of Object, object_new should not be 
_that_ much slower than a g_malloc.

>> Typo, you want deinit. And again, let's avoid type names. This is C,
>> not BASIC.
>
> Ack on the deinit. But see above re: type names.

Yeah, I see now the problem with dependencies, but I'm pretty sure we'll 
reach a compromise. :)

>>> +typedef uint64_t Type;
>>
>> struct TypeImpl;
>> typedef struct TypeImpl *Type;
>
> Not so sure on this.

It doesn't really matter, it can be refactored away later.  But on IRC 
you said it worked out nicely.

The point is when to use strings and when to use type handles.  It 
doesn't matter whether the handles are direct pointers or indices into a 
registry.

>>
>>> +typedef struct ObjectClass ObjectClass;
>>> +typedef struct Object Object;
>>> +
>>> +typedef struct TypeInfo TypeInfo;
>>> +
>>> +typedef struct InterfaceClass InterfaceClass;
>>> +typedef struct Interface Interface;
>>> +typedef struct InterfaceInfo InterfaceInfo;
>>> +
>>> +#define TYPE_OBJECT NULL
>>> +
>>> +/**
>>> + * SECTION:object.h
>>> + * @title:Base Object Type System
>>> + * @short_description: interfaces for creating new types and objects
>>> + *
>>> + * The QEMU Object Model provides a framework for registering user
>>> creatable
>>> + * types and instantiating objects from those types. QOM provides
>>> the following
>>> + * features:
>>> + *
>>> + * - System for dynamically registering types
>>> + * - Support for single-inheritance of types
>>> + * - Multiple inheritance of stateless interfaces
>>> + *
>>> + *<example>
>>> + *<title>Creating a minimal type</title>
>>> + *<programlisting>
>>> + * #include "qdev.h"
>>> + *
>>> + * #define TYPE_MY_DEVICE "my-device"
>>
>> #define TYPE_MY_DEVICE my_device_get_type()
>
> This makes things pretty ugly.

Yes, indeed.  Let's add a fast path for object_new and object_initialize 
that takes a Type value.  To use these you use a get_type() function or 
fetch the Type from a static variable.

Another reason why I'd like you to provide this fast path from the 
beginning, is because you're doing a lot of type_get_by_name() even 
though the type has been found already.  Having the fast paths would 
make you more honest about what needs to do the lookup and what doesn't. 
  Basically only the toplevel calls need to.

Of course this makes this comment moot too:

>>> +#define TYPE_INTERFACE "interface"
>>
>> Let's make TYPE_* constants pointers, not strings.

Thanks,

Paolo

On Wed, Dec 21, 2011 at 08:35:16AM -0600, Anthony Liguori wrote:
> On 12/21/2011 07:35 AM, Paolo Bonzini wrote:
> >What's the need for "Type"? You can use simply the TypeImpl * and drop
> >type_get_instance. Outside object.h it can be an opaque pointer.
> 
> It's a bit nicer for type_register to return a handle that can later
> be unregistered (although that's not currently implemented).
> 
> You could have it return TypeImpl * of course.  GObject uses a
> simpler GType but they don't have the notion of a symbolic type
> name.
> 
> I used a symbolic type name to avoid the problem of dependencies.
> In order to create a type in gobject, you have to reference the
> parent's GType which usually means you have to call the _get_type()
> function which acts as a singleton which registers the type.
> 
> Since you have to specify the parent via a function call, you can't
> define the type in a unit-level static structure which I viewed as a
> critical requirement.

Why not declare types with something like the following:

TypeInfo my_device_info = {
    .name = "my-device",
    .parentinfo = &device_info,
    .instance_size = sizeof(MyDevice),
};

That is, instead of looking up the TypeImpl via a string, lookup the
TypeImpl via the address of the TypeInfo.  (Or possibly store a
pointer to TypeImpl in TypeInfo during registration.)

Module order shouldn't matter - all the info needed to register the
parent is there so it can be registered during first use.  Indeed,
pass a TypeInfo* to object_new() and one should be able to skip the
registration step - if the type hasn't been registered the code can
detect that and automatically register it.

-Kevin

On 12/22/2011 11:25 AM, Kevin O'Connor wrote:
> On Wed, Dec 21, 2011 at 08:35:16AM -0600, Anthony Liguori wrote:
>> On 12/21/2011 07:35 AM, Paolo Bonzini wrote:
>>> What's the need for "Type"? You can use simply the TypeImpl * and drop
>>> type_get_instance. Outside object.h it can be an opaque pointer.
>>
>> It's a bit nicer for type_register to return a handle that can later
>> be unregistered (although that's not currently implemented).
>>
>> You could have it return TypeImpl * of course.  GObject uses a
>> simpler GType but they don't have the notion of a symbolic type
>> name.
>>
>> I used a symbolic type name to avoid the problem of dependencies.
>> In order to create a type in gobject, you have to reference the
>> parent's GType which usually means you have to call the _get_type()
>> function which acts as a singleton which registers the type.
>>
>> Since you have to specify the parent via a function call, you can't
>> define the type in a unit-level static structure which I viewed as a
>> critical requirement.
>
> Why not declare types with something like the following:
>
> TypeInfo my_device_info = {
>      .name = "my-device",
>      .parentinfo =&device_info,
>      .instance_size = sizeof(MyDevice),
> };
>
> That is, instead of looking up the TypeImpl via a string, lookup the
> TypeImpl via the address of the TypeInfo.  (Or possibly store a
> pointer to TypeImpl in TypeInfo during registration.)
>
> Module order shouldn't matter - all the info needed to register the
> parent is there so it can be registered during first use.

The only problem with this is that if a .so implements the type, it means that 
you have to have a way to figure out the dependency order as you load the modules.

OTOH, with the current approach, you can dlopen() all of the modules and 
register the types, and then when the first object is instantiated, that's when 
the type resolution will happen.

This way, modules can be very simple instead of having to encode dependency info 
in a special elf section.

> Indeed,
> pass a TypeInfo* to object_new() and one should be able to skip the
> registration step - if the type hasn't been registered the code can
> detect that and automatically register it.

Yes, I did think of this, the problem is you want to be able to enumerate all of 
the registered types before any objects may be instantiated.  For instance, if 
you did qemu -device ?

Regards,

Anthony Liguori

> -Kevin
>

On Thu, Dec 22, 2011 at 11:41:08AM -0600, Anthony Liguori wrote:
> On 12/22/2011 11:25 AM, Kevin O'Connor wrote:
> >On Wed, Dec 21, 2011 at 08:35:16AM -0600, Anthony Liguori wrote:
> >>I used a symbolic type name to avoid the problem of dependencies.
> >>In order to create a type in gobject, you have to reference the
> >>parent's GType which usually means you have to call the _get_type()
> >>function which acts as a singleton which registers the type.
> >>
> >>Since you have to specify the parent via a function call, you can't
> >>define the type in a unit-level static structure which I viewed as a
> >>critical requirement.
> >
> >Why not declare types with something like the following:
> >
> >TypeInfo my_device_info = {
> >     .name = "my-device",
> >     .parentinfo =&device_info,
> >     .instance_size = sizeof(MyDevice),
> >};
> >
> >That is, instead of looking up the TypeImpl via a string, lookup the
> >TypeImpl via the address of the TypeInfo.  (Or possibly store a
> >pointer to TypeImpl in TypeInfo during registration.)
> >
> >Module order shouldn't matter - all the info needed to register the
> >parent is there so it can be registered during first use.
> 
> The only problem with this is that if a .so implements the type, it
> means that you have to have a way to figure out the dependency order
> as you load the modules.
> 
> OTOH, with the current approach, you can dlopen() all of the modules
> and register the types, and then when the first object is
> instantiated, that's when the type resolution will happen.

If "device_info" is a symbol in basedevice.so and mymodule.so has a
reference to "device_info", wont dlopen("mymodule.so") automatically
bring in basedevice.so?

If not, then it seems like things would get sticky anyway - if the
underlying struct in mymodule.so is defined as:

typedef struct MyDevice
{
    DeviceState parent;

    int reg0, reg1, reg2;
} MyDevice;

it would seem likely that mymodule.so would have assorted references
to basedevice.so for calls on mydev.parent.

I think maybe I'm not understanding the use case.

Cheers,
-Kevin

On 12/22/2011 12:00 PM, Kevin O'Connor wrote:
> On Thu, Dec 22, 2011 at 11:41:08AM -0600, Anthony Liguori wrote:
>> On 12/22/2011 11:25 AM, Kevin O'Connor wrote:
>>> On Wed, Dec 21, 2011 at 08:35:16AM -0600, Anthony Liguori wrote:
>>>> I used a symbolic type name to avoid the problem of dependencies.
>>>> In order to create a type in gobject, you have to reference the
>>>> parent's GType which usually means you have to call the _get_type()
>>>> function which acts as a singleton which registers the type.
>>>>
>>>> Since you have to specify the parent via a function call, you can't
>>>> define the type in a unit-level static structure which I viewed as a
>>>> critical requirement.
>>>
>>> Why not declare types with something like the following:
>>>
>>> TypeInfo my_device_info = {
>>>      .name = "my-device",
>>>      .parentinfo =&device_info,
>>>      .instance_size = sizeof(MyDevice),
>>> };
>>>
>>> That is, instead of looking up the TypeImpl via a string, lookup the
>>> TypeImpl via the address of the TypeInfo.  (Or possibly store a
>>> pointer to TypeImpl in TypeInfo during registration.)
>>>
>>> Module order shouldn't matter - all the info needed to register the
>>> parent is there so it can be registered during first use.
>>
>> The only problem with this is that if a .so implements the type, it
>> means that you have to have a way to figure out the dependency order
>> as you load the modules.
>>
>> OTOH, with the current approach, you can dlopen() all of the modules
>> and register the types, and then when the first object is
>> instantiated, that's when the type resolution will happen.
>
> If "device_info" is a symbol in basedevice.so and mymodule.so has a
> reference to "device_info", wont dlopen("mymodule.so") automatically
> bring in basedevice.so?
>
> If not, then it seems like things would get sticky anyway - if the
> underlying struct in mymodule.so is defined as:
>
> typedef struct MyDevice
> {
>      DeviceState parent;
>
>      int reg0, reg1, reg2;
> } MyDevice;
>
> it would seem likely that mymodule.so would have assorted references
> to basedevice.so for calls on mydev.parent.

Yes, thinking about it, I think you're correct that .parent could refer to some 
sort of type handle.

But I think it's a bit nicer to have a string identify the parent type than an 
extern struct.  I guess it's more a matter of taste than anything else :-)

Regards,

Anthony Liguori

> I think maybe I'm not understanding the use case.
>
> Cheers,
> -Kevin
>

On 12/22/2011 06:25 PM, Kevin O'Connor wrote:
> Why not declare types with something like the following:
>
> TypeInfo my_device_info = {
>      .name = "my-device",
>      .parentinfo =&device_info,
>      .instance_size = sizeof(MyDevice),
> };
>
> That is, instead of looking up the TypeImpl via a string, lookup the
> TypeImpl via the address of the TypeInfo.  (Or possibly store a
> pointer to TypeImpl in TypeInfo during registration.)

This requires all modules to provide a header file just for the TypeInfo 
or TypeImpl.  It doesn't let you use work purely through introspection.

Paolo

On 12/20/2011 05:51 PM, Anthony Liguori wrote:
>   hw/object.c   |  469 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/object.h   |  427 +++++++++++++++++++++++++++++++++++++++++++++++++++

A couple more remarks:

1) Please put it outside hw/

2) Why do you need a list for the interface objects at all?  You can 
make obj->interfaces point to a single blob, and initialize the objects 
in place.  You only need to store the number of interfaces into the 
TypeImpl or in the object header, or add a dummy Interface at the end 
whose contents are all-zero.

Paolo

Am 22.12.2011 20:57, schrieb Anthony Liguori:
> On 12/22/2011 12:00 PM, Kevin O'Connor wrote:
>> On Thu, Dec 22, 2011 at 11:41:08AM -0600, Anthony Liguori wrote:
>>> On 12/22/2011 11:25 AM, Kevin O'Connor wrote:
>>>> Why not declare types with something like the following:
>>>>
>>>> TypeInfo my_device_info = {
>>>>      .name = "my-device",
>>>>      .parentinfo =&device_info,
>>>>      .instance_size = sizeof(MyDevice),
>>>> };
>>>>
>>>> That is, instead of looking up the TypeImpl via a string, lookup the
>>>> TypeImpl via the address of the TypeInfo.  (Or possibly store a
>>>> pointer to TypeImpl in TypeInfo during registration.)
[...]
> Yes, thinking about it, I think you're correct that .parent could refer
> to some sort of type handle.
> 
> But I think it's a bit nicer to have a string identify the parent type
> than an extern struct.  I guess it's more a matter of taste than
> anything else :-)

The advantage of using some kind of symbolic name is that the compiler
helps us avoid typos. If we use strings, we defer the checking to the
actual instantiation at runtime.

Andreas

On 01/02/2012 05:01 PM, Andreas Färber wrote:
> Am 22.12.2011 20:57, schrieb Anthony Liguori:
>> On 12/22/2011 12:00 PM, Kevin O'Connor wrote:
>>> On Thu, Dec 22, 2011 at 11:41:08AM -0600, Anthony Liguori wrote:
>>>> On 12/22/2011 11:25 AM, Kevin O'Connor wrote:
>>>>> Why not declare types with something like the following:
>>>>>
>>>>> TypeInfo my_device_info = {
>>>>>       .name = "my-device",
>>>>>       .parentinfo =&device_info,
>>>>>       .instance_size = sizeof(MyDevice),
>>>>> };
>>>>>
>>>>> That is, instead of looking up the TypeImpl via a string, lookup the
>>>>> TypeImpl via the address of the TypeInfo.  (Or possibly store a
>>>>> pointer to TypeImpl in TypeInfo during registration.)
> [...]
>> Yes, thinking about it, I think you're correct that .parent could refer
>> to some sort of type handle.
>>
>> But I think it's a bit nicer to have a string identify the parent type
>> than an extern struct.  I guess it's more a matter of taste than
>> anything else :-)
>
> The advantage of using some kind of symbolic name is that the compiler
> helps us avoid typos. If we use strings, we defer the checking to the
> actual instantiation at runtime.

That's why I use #define's to make a symbol for the types.  The only reason 
that's not done universally is it would be a bit challenging to script the 
conversion to do that (I though I did attempt it a couple times).

Regards,

Anthony Liguori

>
> Andreas
>

On 01/02/2012 11:59 AM, Paolo Bonzini wrote:
> On 12/20/2011 05:51 PM, Anthony Liguori wrote:
>> hw/object.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> hw/object.h | 427 +++++++++++++++++++++++++++++++++++++++++++++++++++
>
> A couple more remarks:
>
> 1) Please put it outside hw/

Done :-)

>
> 2) Why do you need a list for the interface objects at all? You can make
> obj->interfaces point to a single blob, and initialize the objects in place. You
> only need to store the number of interfaces into the TypeImpl or in the object
> header, or add a dummy Interface at the end whose contents are all-zero.

Interfaces aren't really handled in a special way.  You could easily make it so 
that the infrastructure could be modified to allow true multiple inheritance.

I'm not sure I see an obvious benefit to making interfaces handled more as a 
special case.  Right now, the only place that really even knows about interfaces 
is dynamic_cast and initialization.  That seems like a good thing to me.

Regards,

Anthony Liguori

> Paolo
>

On 01/03/2012 02:18 AM, Anthony Liguori wrote:
>>
>> 2) Why do you need a list for the interface objects at all? You can make
>> obj->interfaces point to a single blob, and initialize the objects in
>> place. You
>> only need to store the number of interfaces into the TypeImpl or in
>> the object
>> header, or add a dummy Interface at the end whose contents are all-zero.
>
> Interfaces aren't really handled in a special way.  You could easily
> make it so that the infrastructure could be modified to allow true
> multiple inheritance.
>
> I'm not sure I see an obvious benefit to making interfaces handled more
> as a special case.  Right now, the only place that really even knows
> about interfaces is dynamic_cast and initialization.  That seems like a
> good thing to me.

That wouldn't really be a special case, not any more than initializing a 
device's child in-place within the parent struct.

Anyhow, nothing that can't be done as a followup.  Will review v2 now.

Paolo