Il 13/06/2012 11:38, Peter A. G. Crosthwaite ha scritto: > Something is broken with casting to an interface type then setting a link. > Cast > these two to object for linking instead and it all works.
I don't have a board image but I don't see anything visibly bad without this patch. However, something that _is_ bad indeed happens; we try to ref/unref an interface object. This patch fixes it while keeping things efficient (and in fact fixes one TODO). Can add a link to an image on http://wiki.qemu.org/Testing and/or test it? Anthony, is this above your disgust level? Paolo ------------------------ 8< ----------------------- diff --git a/qom/object.c b/qom/object.c index c3a7a47..bd60838 100644 --- a/qom/object.c +++ b/qom/object.c @@ -63,7 +63,7 @@ struct TypeImpl InterfaceImpl interfaces[MAX_INTERFACES]; }; -#define INTERFACE(obj) INTERFACE_CHECK(obj, TYPE_INTERFACE) +#define INTERFACE(obj) ((Interface *)(obj)) static Type type_interface; @@ -239,13 +239,21 @@ static void type_initialize(TypeImpl *ti) } } +#define INTERFACE_MAGIC ((GSList *) (intptr_t)0xBAD0BAD) + +static inline bool object_is_interface(Object *obj) { + return obj->interfaces == INTERFACE_MAGIC; +} + static void object_interface_init(Object *obj, InterfaceImpl *iface) { TypeImpl *ti = iface->type; Interface *iface_obj; + assert(!object_is_interface(obj)); iface_obj = INTERFACE(object_new(ti->name)); iface_obj->iface_obj = obj; + iface_obj->iface_parent.interfaces = INTERFACE_MAGIC; obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj); } @@ -332,10 +340,12 @@ 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 (!object_is_interface(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)) { @@ -410,6 +420,13 @@ Object *object_dynamic_cast(Object *obj, const char *typename) TypeImpl *target_type = type_get_by_name(typename); GSList *i; + /* Check if obj is an interface and its containing object is a direct + * ancestor of typename. object_is_interface is a very fast test. + */ + if (object_is_interface(obj)) { + obj = INTERFACE(obj)->iface_obj; + } + /* Check if typename is a direct ancestor. Special-case TYPE_OBJECT, * we want to go back from interfaces to the parent. */ @@ -417,24 +434,6 @@ 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)->iface_obj; - if (object_is_type(obj, target_type)) { - return obj; - } - } - if (!target_type) { return obj; } @@ -597,11 +596,19 @@ GSList *object_class_get_list(const char *implements_type, void object_ref(Object *obj) { + if (object_is_interface(obj)) { + obj = INTERFACE(obj)->iface_obj; + } + obj->ref++; } void object_unref(Object *obj) { + if (object_is_interface(obj)) { + obj = INTERFACE(obj)->iface_obj; + } + g_assert(obj->ref > 0); obj->ref--; @@ -979,7 +986,7 @@ gchar *object_get_canonical_path(Object *obj) Object *root = object_get_root(); char *newpath = NULL, *path = NULL; - if (object_is_type(obj, type_interface)) { + if (object_is_interface(obj)) { obj = INTERFACE(obj)->iface_obj; } Paolo
