From patchwork Sat Jun 16 10:39:01 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Peter A. G. Crosthwaite" X-Patchwork-Id: 165285 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 47B56B712C for ; Sat, 16 Jun 2012 20:29:51 +1000 (EST) Received: from localhost ([::1]:33325 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfqGL-0006zC-4V for incoming@patchwork.ozlabs.org; Sat, 16 Jun 2012 06:29:49 -0400 Received: from eggs.gnu.org ([208.118.235.92]:51653) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfqG8-0006nh-F8 for qemu-devel@nongnu.org; Sat, 16 Jun 2012 06:29:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfqG5-0005Ta-Gx for qemu-devel@nongnu.org; Sat, 16 Jun 2012 06:29:36 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:33900) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfqG5-0005TQ-6T for qemu-devel@nongnu.org; Sat, 16 Jun 2012 06:29:33 -0400 Received: by pbbro12 with SMTP id ro12so6616730pbb.4 for ; Sat, 16 Jun 2012 03:29:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references :in-reply-to:references:x-gm-message-state; bh=MJPQW5cKRg4M9YTMcAEr/sLFiWzP5HWzV8jg9PzPwvk=; b=L3P6N46a+3h6kuHym0FbKJXT4iQ4MMreAE4Z0dBkhAptWIKe8bG4P5oc80a+F1lncu 9Ni3XRZ/oN6GBUGIB659JjUYriW7utbykjY+hi/xMbm9wceoXz7VhF1i3pBt1eKjxfHE Kd1NyLu021/aiVw+4iNHPJYfqLV4WHDZeQAMm8yzZPoAw0vTki3nMEhiWd97QZyjR3d2 Zd+F42a71G3svnUTSNyn7IG1Zttm79u/CISo75PhvjCeeqboDVrNK6N5PUQA4Y6DHwHb oJtKaOnPBn1GSf7eLWOwkHeca/T1OMSY3F1LVdK5+2G0k4F/9VYlDP94AagMFq2M0buz ZuPw== Received: by 10.68.202.136 with SMTP id ki8mr28653755pbc.65.1339842570558; Sat, 16 Jun 2012 03:29:30 -0700 (PDT) Received: from localhost ([124.148.20.9]) by mx.google.com with ESMTPS id rv5sm16420903pbc.56.2012.06.16.03.29.25 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 16 Jun 2012 03:29:29 -0700 (PDT) From: "Peter A. G. Crosthwaite" To: qemu-devel@nongnu.org, edgar.iglesias@gmail.com, aliguori@us.ibm.com, pbonzini@redhat.com Date: Sat, 16 Jun 2012 20:39:01 +1000 Message-Id: X-Mailer: git-send-email 1.7.3.2 In-Reply-To: References: In-Reply-To: References: X-Gm-Message-State: ALoCoQn1x6LlvaI7V3x2PslU3ovbAmqunYJP0ZIE/cEEs+320qXwFB7rSsWOoUZA1P3t1ywCJqip X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.160.45 Cc: peter.maydell@linaro.org, peter.crosthwaite@petalogix.com, paul@codesourcery.com, afaerber@suse.de, john.williams@petalogix.com, avi@redhat.com Subject: [Qemu-devel] [PATCH v1 1/3] qom: reimplement Interfaces X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Anthony Liguori The current implementation of Interfaces is poorly designed. Each interface that an object implements end up being an object that's tracked by the implementing object. There's all sorts of gymnastics to deal with casting between these objects. By an interface shouldn't be associated with an Object. Interfaces are global to a class. This patch moves all Interface knowledge to ObjectClass eliminating the relationship between Object and Interfaces. Interfaces are now abstract (as they should be) but this is okay. Interfaces essentially act as additional parents for the classes and are treated as such. With this new implementation, we should fully support derived interfaces including reimplementing an inherited interface. Signed-off-by: Anthony Liguori --- include/qemu/object.h | 64 +++++++++++--- qom/object.c | 231 +++++++++++++++++++++++------------------------- 2 files changed, 160 insertions(+), 135 deletions(-) diff --git a/include/qemu/object.h b/include/qemu/object.h index d93b772..72cb290 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -239,6 +239,7 @@ struct ObjectClass { /*< private >*/ Type type; + GSList *interfaces; }; /** @@ -260,7 +261,6 @@ struct Object { /*< private >*/ ObjectClass *class; - GSList *interfaces; QTAILQ_HEAD(, ObjectProperty) properties; uint32_t ref; Object *parent; @@ -381,6 +381,17 @@ struct TypeInfo OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name) /** + * InterfaceInfo: + * @type: The name of the interface. + * + * The information associated with an interface. + */ +struct InterfaceInfo +{ + const char *type; +}; + +/** * InterfaceClass: * @parent_class: the base class * @@ -390,26 +401,31 @@ struct TypeInfo struct InterfaceClass { ObjectClass parent_class; + /*< private >*/ + ObjectClass *concrete_class; }; +#define TYPE_INTERFACE "interface" + /** - * 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. + * INTERFACE_CLASS: + * @klass: class to cast from * - * The information associated with an interface. + * Returns: An #InterfaceClass or raise an error if cast is invalid */ -struct InterfaceInfo -{ - const char *type; +#define INTERFACE_CLASS(klass) \ + OBJECT_CLASS_CHECK(InterfaceClass, klass, TYPE_INTERFACE) - void (*interface_initfn)(ObjectClass *class, void *data); -}; - -#define TYPE_INTERFACE "interface" +/** + * INTERFACE_CHECK: + * @interface: the type to return + * @obj: the object to convert to an interface + * @name: the interface type name + * + * Returns: @obj casted to @interface if cast is valid, otherwise raise error. + */ +#define INTERFACE_CHECK(interface, obj, name) \ + ((interface *)interface_dynamic_cast_assert(OBJECT((obj)), (name))) /** * object_new: @@ -548,6 +564,24 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *klass, const char *typename); /** + * interface_dynamic_cast_assert: + * @obj: the object to cast to an interface type + * @typename: the type name of the interface + * + * Returns: @obj if @obj implements @typename, otherwise raise an error + */ +Object *interface_dynamic_cast_assert(Object *obj, const char *typename); + +/** + * interface_dynamic_cast_assert: + * @obj: the object to cast to an interface type + * @typename: the type name of the interface + * + * Returns: @obj if @obj implements @typename, otherwise %NULL + */ +Object *interface_dynamic_cast(Object *obj, const char *typename); + +/** * object_class_get_name: * @klass: The class to obtain the QOM typename for. * diff --git a/qom/object.c b/qom/object.c index 6f839ad..aa26693 100644 --- a/qom/object.c +++ b/qom/object.c @@ -31,9 +31,7 @@ typedef struct TypeImpl TypeImpl; struct InterfaceImpl { - const char *parent; - void (*interface_initfn)(ObjectClass *class, void *data); - TypeImpl *type; + const char *typename; }; struct TypeImpl @@ -63,14 +61,6 @@ struct TypeImpl InterfaceImpl interfaces[MAX_INTERFACES]; }; -typedef struct Interface -{ - Object parent; - Object *obj; -} Interface; - -#define INTERFACE(obj) OBJECT_CHECK(Interface, obj, TYPE_INTERFACE) - static Type type_interface; static GHashTable *type_table_get(void) @@ -97,6 +87,7 @@ static TypeImpl *type_table_lookup(const char *name) TypeImpl *type_register(const TypeInfo *info) { TypeImpl *ti = g_malloc0(sizeof(*ti)); + int i; g_assert(info->name != NULL); @@ -120,15 +111,10 @@ TypeImpl *type_register(const TypeInfo *info) 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++; - } + for (i = 0; info->interfaces && info->interfaces[i].type; i++) { + ti->interfaces[i].typename = g_strdup(info->interfaces[i].type); } + ti->num_interfaces = i; type_table_add(ti); @@ -190,26 +176,48 @@ static size_t type_object_get_size(TypeImpl *ti) return 0; } -static void type_class_interface_init(TypeImpl *ti, InterfaceImpl *iface) +static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type) { - TypeInfo info = { - .instance_size = sizeof(Interface), - .parent = iface->parent, - .class_size = sizeof(InterfaceClass), - .class_init = iface->interface_initfn, - .abstract = true, - }; - char *name = g_strdup_printf("<%s::%s>", ti->name, iface->parent); + assert(target_type); + + /* Check if typename is a direct ancestor of type */ + while (type) { + if (type == target_type) { + return true; + } + + type = type_get_parent(type); + } + + return false; +} + +static void type_initialize(TypeImpl *ti); + +static void type_initialize_interface(TypeImpl *ti, const char *parent) +{ + InterfaceClass *new_iface; + TypeInfo info = { }; + TypeImpl *iface_impl; - info.name = name; - iface->type = type_register(&info); - g_free(name); + info.parent = parent; + info.name = g_strdup_printf("%s::%s", ti->name, info.parent); + info.abstract = true; + + iface_impl = type_register(&info); + type_initialize(iface_impl); + g_free((char *)info.name); + + new_iface = (InterfaceClass *)iface_impl->class; + new_iface->concrete_class = ti->class; + + ti->class->interfaces = g_slist_append(ti->class->interfaces, + iface_impl->class); } static void type_initialize(TypeImpl *ti) { size_t class_size = sizeof(ObjectClass); - int i; if (ti->class) { return; @@ -221,8 +229,12 @@ static void type_initialize(TypeImpl *ti) ti->class = g_malloc0(ti->class_size); ti->class->type = ti; + ti->class->interfaces = NULL; + if (type_has_parent(ti)) { TypeImpl *parent = type_get_parent(ti); + GSList *e; + int i; type_initialize(parent); @@ -232,42 +244,46 @@ static void type_initialize(TypeImpl *ti) memcpy((void *)ti->class + sizeof(ObjectClass), (void *)parent->class + sizeof(ObjectClass), parent->class_size - sizeof(ObjectClass)); - } - memset((void *)ti->class + class_size, 0, ti->class_size - class_size); + for (e = parent->class->interfaces; e; e = e->next) { + ObjectClass *iface = e->data; + type_initialize_interface(ti, object_class_get_name(iface)); + } - for (i = 0; i < ti->num_interfaces; i++) { - type_class_interface_init(ti, &ti->interfaces[i]); - } + for (i = 0; i < ti->num_interfaces; i++) { + TypeImpl *t = type_get_by_name(ti->interfaces[i].typename); - if (ti->class_init) { - ti->class_init(ti->class, ti->class_data); + for (e = ti->class->interfaces; e; e = e->next) { + TypeImpl *target_type = OBJECT_CLASS(e->data)->type; + + if (type_is_ancestor(target_type, t)) { + break; + } + } + + if (e) { + continue; + } + + type_initialize_interface(ti, ti->interfaces[i].typename); + } } -} -static void object_interface_init(Object *obj, InterfaceImpl *iface) -{ - TypeImpl *ti = iface->type; - Interface *iface_obj; + /* If this type is an interface and num_interfaces, bugger out */ - iface_obj = INTERFACE(object_new(ti->name)); - iface_obj->obj = obj; + memset((void *)ti->class + class_size, 0, ti->class_size - class_size); - obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj); + if (ti->class_init) { + ti->class_init(ti->class, ti->class_data); + } } static void object_init_with_type(Object *obj, TypeImpl *ti) { - int i; - if (type_has_parent(ti)) { object_init_with_type(obj, type_get_parent(ti)); } - for (i = 0; i < ti->num_interfaces; i++) { - object_interface_init(obj, &ti->interfaces[i]); - } - if (ti->instance_init) { ti->instance_init(obj); } @@ -338,12 +354,6 @@ static void object_deinit(Object *obj, TypeImpl *type) type->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 (type_has_parent(type)) { object_deinit(obj, type_get_parent(type)); } @@ -390,22 +400,6 @@ void object_delete(Object *obj) g_free(obj); } -static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type) -{ - assert(target_type); - - /* Check if typename is a direct ancestor of type */ - while (type) { - if (type == target_type) { - return true; - } - - type = type_get_parent(type); - } - - return false; -} - static bool object_is_type(Object *obj, TypeImpl *target_type) { return !target_type || type_is_ancestor(obj->class->type, target_type); @@ -414,7 +408,6 @@ static bool object_is_type(Object *obj, TypeImpl *target_type) Object *object_dynamic_cast(Object *obj, const char *typename) { TypeImpl *target_type = type_get_by_name(typename); - GSList *i; /* Check if typename is a direct ancestor. Special-case TYPE_OBJECT, * we want to go back from interfaces to the parent. @@ -423,46 +416,18 @@ Object *object_dynamic_cast(Object *obj, const char *typename) return obj; } - /* Check if obj is an interface and its containing object is a direct - * ancestor of typename. In principle we could do this test at the very - * beginning of object_dynamic_cast, avoiding a second call to - * object_is_type. However, casting between interfaces is relatively - * rare, and object_is_type(obj, type_interface) would fail almost always. - * - * Perhaps we could add a magic value to the object header for increased - * (run-time) type safety and to speed up tests like this one. If we ever - * do that we can revisit the order here. - */ - if (object_is_type(obj, type_interface)) { - assert(!obj->interfaces); - obj = INTERFACE(obj)->obj; - if (object_is_type(obj, target_type)) { - return obj; - } - } - if (!target_type) { 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), target_type)) { - return OBJECT(iface); - } - } - return NULL; } - static void register_types(void) { static TypeInfo interface_info = { .name = TYPE_INTERFACE, - .instance_size = sizeof(Interface), + .class_size = sizeof(InterfaceClass), .abstract = true, }; @@ -491,16 +456,30 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class, { TypeImpl *target_type = type_get_by_name(typename); TypeImpl *type = class->type; + ObjectClass *ret = NULL; - while (type) { - if (type == target_type) { - return class; + if (type->num_interfaces && type_is_ancestor(target_type, type_interface)) { + int found = 0; + GSList *i; + + for (i = class->interfaces; i; i = i->next) { + ObjectClass *target_class = i->data; + + if (type_is_ancestor(target_class->type, target_type)) { + ret = target_class; + found++; + } } - type = type_get_parent(type); + /* The match was ambiguous, don't allow a cast */ + if (found > 1) { + ret = NULL; + } + } else if (type_is_ancestor(type, target_type)) { + ret = class; } - return NULL; + return ret; } ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class, @@ -517,6 +496,28 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class, return ret; } +Object *interface_dynamic_cast(Object *obj, const char *typename) +{ + if (object_class_dynamic_cast(object_get_class(obj), typename)) { + return obj; + } + + return NULL; +} + +Object *interface_dynamic_cast_assert(Object *obj, const char *typename) +{ + Object *ret = interface_dynamic_cast(obj, typename); + + if (!ret) { + fprintf(stderr, "Object %p is not an instance of type %s\n", + obj, typename); + abort(); + } + + return ret; +} + const char *object_get_typename(Object *obj) { return obj->class->type->name; @@ -883,12 +884,6 @@ void object_property_add_child(Object *obj, const char *name, { gchar *type; - /* Registering an interface object in the composition tree will mightily - * confuse object_get_canonical_path (which, on the other hand, knows how - * to get the canonical path of an interface object). - */ - assert(!object_is_type(obj, type_interface)); - type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child))); object_property_add(obj, name, type, object_get_child_property, @@ -985,10 +980,6 @@ gchar *object_get_canonical_path(Object *obj) Object *root = object_get_root(); char *newpath = NULL, *path = NULL; - if (object_is_type(obj, type_interface)) { - obj = INTERFACE(obj)->obj; - } - while (obj != root) { ObjectProperty *prop = NULL;