From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34946) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPH9-0002R2-M5 for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:10:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RuPH4-00079m-D8 for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:10:35 -0500 Received: from mail-pz0-f45.google.com ([209.85.210.45]:57945) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPH4-00079Z-8U for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:10:30 -0500 Received: by dadp14 with SMTP id p14so6469470dad.4 for ; Mon, 06 Feb 2012 06:10:29 -0800 (PST) Message-ID: <4F2FDF51.1060503@codemonkey.ws> Date: Mon, 06 Feb 2012 08:10:25 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1328342577-25732-1-git-send-email-pbonzini@redhat.com> <1328342577-25732-4-git-send-email-pbonzini@redhat.com> In-Reply-To: <1328342577-25732-4-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 03/27] qom: clean up/optimize object_dynamic_cast List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 02/04/2012 02:02 AM, Paolo Bonzini wrote: > The interface loop can be performed only on the parent object. It > does not need to be done on each interface. Similarly, we can > simplify the code by switching early from the implementation > object to the parent object. > > Signed-off-by: Paolo Bonzini > --- > qom/object.c | 38 ++++++++++++++++++-------------------- > 1 files changed, 18 insertions(+), 20 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 4261944..4d21f0a 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -372,7 +372,6 @@ static bool object_is_type(Object *obj, const char *typename) > { > TypeImpl *target_type = type_get_by_name(typename); > TypeImpl *type = obj->class->type; > - GSList *i; > > /* Check if typename is a direct ancestor of type */ > while (type) { > @@ -383,15 +382,6 @@ static bool object_is_type(Object *obj, const char *typename) > type = type_get_parent(type); > } > > - /* 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; > } So this changes object_is_type() to only determine if the the object is a direct ancestor. I think it's worth changing the name to something like object_is_ancestor_of() or something like that. > > @@ -404,6 +394,24 @@ 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, typename)) { > + return obj; > + } > + } > + > /* Check if obj has an interface of typename */ > for (i = obj->interfaces; i; i = i->next) { > Interface *iface = i->data; > @@ -413,16 +421,6 @@ Object *object_dynamic_cast(Object *obj, const char *typename) > } > } > > - /* Check if obj is an interface and its 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; > } This looks pretty right to me. We really need a unit test for this stuff. I've got one in my tree, I'll dig it out so we can start more rigorously validating these different casting cases. Regards, Anthony Liguori >