Re: [Qemu-devel] [PATCH 04/10] qom: add get_id

["Andreas Färber" <afaerber@suse.de>]

Am 23.05.2012 17:44, schrieb Paolo Bonzini:
> Some classes may present objects differently in errors, for example if they
> are not part of the composition tree or if they are not assigned an id by
> the user.  Let them do this with a get_id method on Object, and use the
> method consistently where a %(device) appears in the error.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

This patch looks new, so it's not covered by Anthony's carte blanche to
take Paolo's patches into qom-next.

Generally I'm fine with it, some formal issues below.

> ---
>  hw/qdev-properties.c  |    6 +++---
>  hw/qdev.c             |   15 ++++++++++++++-
>  include/qemu/object.h |   11 +++++++++++
>  qom/object.c          |   26 +++++++++++++++++++++++++-
>  4 files changed, 53 insertions(+), 5 deletions(-)

> diff --git a/hw/qdev.c b/hw/qdev.c
> index e909f3b..5d6dc1f 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
[...]
> @@ -714,6 +714,13 @@ static void device_finalize(Object *obj)
>      }
>  }
>  
> +static const char *qdev_get_id(Object *obj)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +
> +    return dev->id?:object_get_typename(obj);

Spaces around ?: please.

> +}
> +
>  static void device_class_base_init(ObjectClass *class, void *data)
>  {
>      DeviceClass *klass = DEVICE_CLASS(class);
[...]
> diff --git a/qom/object.c b/qom/object.c
> index 68a4c57..b19ef94 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -334,6 +334,24 @@ static void object_property_del_child(Object *obj, Object *child, Error **errp)
>      }
>  }
>  
> +static const char *_object_get_id(Object *obj)

malc's alarm bells will be ringing: Use of an identifier starting with
underscore.

What about object_get_default_id()?

> +{
> +    ObjectProperty *prop;
> +
> +    QTAILQ_FOREACH(prop, &obj->properties, node) {
> +        if (strstart(prop->type, "child<", NULL) && prop->opaque == obj) {

At one point we were discussing an object_property_is_child() inline
helper. I'd really prefer not to spread string comparisons everywhere.
I'll submit a patch based on this one:

http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02549.html

Andreas

> +            return prop->name;
> +        }
> +    }
> +
> +    return "";
> +}
> +
> +const char *object_get_id(Object *obj)
> +{
> +    return obj->class->get_id(obj);
> +}
> +
>  void object_unparent(Object *obj)
>  {
>      if (obj->parent) {
[...]
> @@ -1236,6 +1254,11 @@ static void object_instance_init(Object *obj)
>      object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
>  }
>  
> +static void object_class_init(ObjectClass *klass, void *class_data)
> +{
> +    klass->get_id = _object_get_id;
> +}
> +
>  static void register_types(void)
>  {
>      static TypeInfo interface_info = {
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg