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().
>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> include/qom/object.h | 8 ++++++++
> qom/object.c | 53
> ++++++++++++++++++++++++++++++----------------------
> 2 files changed, 39 insertions(+), 22 deletions(-)
>
> 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);
>
> /**
> + * object_get_canonical_basename:
> + *
> + * Returns: The final component in the object's canonical path. The
> canonical
> + * 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, const char
> *name,
> g_free(full_type);
> }
>
> +gchar *object_get_canonical_basename(Object *obj)
> +{
> + ObjectProperty *prop = NULL;
> +
> + g_assert(obj->parent != 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 == 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 = object_get_root();
> - char *newpath = NULL, *path = NULL;
> + char *newpath, *path = NULL;
>
> while (obj != root) {
> - ObjectProperty *prop = NULL;
> -
> - g_assert(obj->parent != NULL);
> -
> - QTAILQ_FOREACH(prop, &obj->parent->properties, node) {
> - if (!object_property_is_child(prop)) {
> - continue;
> - }
> + char *component = object_get_canonical_basename(obj);
>
> - if (prop->opaque == obj) {
> - if (path) {
> - newpath = g_strdup_printf("%s/%s", prop->name, path);
> - g_free(path);
> - path = newpath;
> - } else {
> - path = g_strdup(prop->name);
> - }
> - break;
> - }
> + if (path) {
> + newpath = g_strdup_printf("%s/%s", component, path);
> + g_free(component);
> + g_free(path);
> + path = newpath;
> + } else {
> + path = component;
> }
>
> - g_assert(prop != NULL);
> -
> obj = obj->parent;
> }
>
> - newpath = g_strdup_printf("/%s", path);
> + newpath = g_strdup_printf("/%s", path ? path : "");
> g_free(path);
>
> return newpath;
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
