From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJR2c-0005TN-7U for qemu-devel@nongnu.org; Fri, 28 Feb 2014 12:16:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WJR2U-0002y9-Ts for qemu-devel@nongnu.org; Fri, 28 Feb 2014 12:16:06 -0500 Received: from cantor2.suse.de ([195.135.220.15]:57178 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJR2U-0002xu-IR for qemu-devel@nongnu.org; Fri, 28 Feb 2014 12:15:58 -0500 Message-ID: <5310C44B.1050302@suse.de> Date: Fri, 28 Feb 2014 18:15:55 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1393600696-24118-1-git-send-email-stefanha@redhat.com> <1393600696-24118-2-git-send-email-stefanha@redhat.com> In-Reply-To: <1393600696-24118-2-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5 1/7] object: add object_get_canonical_basename() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Kevin Wolf , Paolo Bonzini , Fam Zheng , Michael Roth , Igor Mammedov Am 28.02.2014 16:18, schrieb Stefan Hajnoczi: > It is often useful to find an object's child property name. Also use > this new function to simplify the implementation of > object_get_canonical_path(). >=20 > Signed-off-by: Stefan Hajnoczi > --- > include/qom/object.h | 8 ++++++++ > qom/object.c | 53 ++++++++++++++++++++++++++++++--------------= -------- > 2 files changed, 39 insertions(+), 22 deletions(-) >=20 > diff --git a/include/qom/object.h b/include/qom/object.h > index 9c7c361..8c6db7c 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -974,6 +974,14 @@ const char *object_property_get_type(Object *obj, = const char *name, > Object *object_get_root(void); > =20 > /** > + * object_get_canonical_basename: > + * > + * Returns: The final component in the object's canonical path. The c= anonical > + * path is the path within the composition tree starting from the root= . > + */ > +gchar *object_get_canonical_basename(Object *obj); I find this name very confusing. There is no such thing as a canonical base name, ..._canonical_path_component would make its purpose much more obvious. An underlying issue here probably is that Anthony didn't want a public API to access the Object::parent. But the only other user is iothread_get_id() in 4/7, so we might just loop over its known path prefix there to discover the right child<>. On the other hand, couldn't device IDs in /machine/peripheral benefit from this today, too? In any way I would've liked to get CC'ed on this QOM API proposal please. > + > +/** > * object_get_canonical_path: > * > * Returns: The canonical path for a object. This is the path within = the > diff --git a/qom/object.c b/qom/object.c > index 660859c..0cdc319 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1102,39 +1102,48 @@ void object_property_add_link(Object *obj, cons= t char *name, > g_free(full_type); > } > =20 > +gchar *object_get_canonical_basename(Object *obj) > +{ > + ObjectProperty *prop =3D NULL; > + > + g_assert(obj->parent !=3D NULL); It might make sense to assert obj first? Accessing ->parent would not be much different from ->parent->properties otherwise. But applies to the original code as well and the movement looks OK. Regards, Andreas > + > + QTAILQ_FOREACH(prop, &obj->parent->properties, node) { > + if (!object_property_is_child(prop)) { > + continue; > + } > + > + if (prop->opaque =3D=3D obj) { > + return g_strdup(prop->name); > + } > + } > + > + /* obj had a parent but was not a child, should never happen */ > + g_assert_not_reached(); > + return NULL; > +} > + > gchar *object_get_canonical_path(Object *obj) > { > Object *root =3D object_get_root(); > - char *newpath =3D NULL, *path =3D NULL; > + char *newpath, *path =3D NULL; > =20 > while (obj !=3D root) { > - ObjectProperty *prop =3D NULL; > - > - g_assert(obj->parent !=3D NULL); > - > - QTAILQ_FOREACH(prop, &obj->parent->properties, node) { > - if (!object_property_is_child(prop)) { > - continue; > - } > + char *component =3D object_get_canonical_basename(obj); > =20 > - if (prop->opaque =3D=3D obj) { > - if (path) { > - newpath =3D g_strdup_printf("%s/%s", prop->name, p= ath); > - g_free(path); > - path =3D newpath; > - } else { > - path =3D g_strdup(prop->name); > - } > - break; > - } > + if (path) { > + newpath =3D g_strdup_printf("%s/%s", component, path); > + g_free(component); > + g_free(path); > + path =3D newpath; > + } else { > + path =3D component; > } > =20 > - g_assert(prop !=3D NULL); > - > obj =3D obj->parent; > } > =20 > - newpath =3D g_strdup_printf("/%s", path); > + newpath =3D g_strdup_printf("/%s", path ? path : ""); > g_free(path); > =20 > return newpath; --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg