Re: [Qemu-devel] [PATCH v2 01/10] qom: make user_creatable_complete() specific to UserCreatable

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 30 Oct 2018 19:04:44 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> Instead of accepting any Object*, change user_creatable_complete() to
> require a UserCreatable*. Modify the callers to pass the appropriate
> argument, removing redundant dynamic cast checks in object creation.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Nice patch, it also reduces number of dynamic casts originated from
object_initialize_childv/object_new_with_propv.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/qom/object_interfaces.h |  4 ++--
>  hw/misc/ivshmem.c               |  2 +-
>  hw/virtio/virtio-rng.c          |  2 +-
>  qom/object.c                    | 12 ++++++++----
>  qom/object_interfaces.c         | 14 +++-----------
>  5 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
> index 4d513fb329..7450cff908 100644
> --- a/include/qom/object_interfaces.h
> +++ b/include/qom/object_interfaces.h
> @@ -55,14 +55,14 @@ typedef struct UserCreatableClass {
>  
>  /**
>   * user_creatable_complete:
> - * @obj: the object whose complete() method is called if defined
> + * @uc: the user-creatable object whose complete() method is called if defined
>   * @errp: if an error occurs, a pointer to an area to store the error
>   *
>   * Wrapper to call complete() method if one of types it's inherited
>   * from implements USER_CREATABLE interface, otherwise the call does
>   * nothing.
>   */
> -void user_creatable_complete(Object *obj, Error **errp);
> +void user_creatable_complete(UserCreatable *uc, Error **errp);
>  
>  /**
>   * user_creatable_can_be_deleted:
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index f88910e55c..478f41044c 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -1279,7 +1279,7 @@ static void desugar_shm(IVShmemState *s)
>      object_property_set_bool(obj, true, "share", &error_abort);
>      object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
>                                &error_abort);
> -    user_creatable_complete(obj, &error_abort);
> +    user_creatable_complete(USER_CREATABLE(obj), &error_abort);
>      s->hostmem = MEMORY_BACKEND(obj);
>  }
>  
> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> index 855f1b41d1..30493a2586 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -191,7 +191,7 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp)
>      if (vrng->conf.rng == NULL) {
>          vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM));
>  
> -        user_creatable_complete(OBJECT(vrng->conf.default_backend),
> +        user_creatable_complete(USER_CREATABLE(vrng->conf.default_backend),
>                                  &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
> diff --git a/qom/object.c b/qom/object.c
> index 547dcf97c3..eb770dbf7f 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -417,6 +417,7 @@ void object_initialize_childv(Object *parentobj, const char *propname,
>  {
>      Error *local_err = NULL;
>      Object *obj;
> +    UserCreatable *uc;
>  
>      object_initialize(childobj, size, type);
>      obj = OBJECT(childobj);
> @@ -431,8 +432,9 @@ void object_initialize_childv(Object *parentobj, const char *propname,
>          goto out;
>      }
>  
> -    if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
> -        user_creatable_complete(obj, &local_err);
> +    uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
> +    if (uc) {
> +        user_creatable_complete(uc, &local_err);
>          if (local_err) {
>              object_unparent(obj);
>              goto out;
> @@ -590,6 +592,7 @@ Object *object_new_with_propv(const char *typename,
>      Object *obj;
>      ObjectClass *klass;
>      Error *local_err = NULL;
> +    UserCreatable *uc;
>  
>      klass = object_class_by_name(typename);
>      if (!klass) {
> @@ -612,8 +615,9 @@ Object *object_new_with_propv(const char *typename,
>          goto error;
>      }
>  
> -    if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
> -        user_creatable_complete(obj, &local_err);
> +    uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
> +    if (uc) {
> +        user_creatable_complete(uc, &local_err);
>          if (local_err) {
>              object_unparent(obj);
>              goto error;
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 97b79b48bb..db85d1eb75 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -8,18 +8,10 @@
>  #include "qapi/opts-visitor.h"
>  #include "qemu/config-file.h"
>  
> -void user_creatable_complete(Object *obj, Error **errp)
> +void user_creatable_complete(UserCreatable *uc, Error **errp)
>  {
> +    UserCreatableClass *ucc = USER_CREATABLE_GET_CLASS(uc);
>  
> -    UserCreatableClass *ucc;
> -    UserCreatable *uc =
> -        (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
> -
> -    if (!uc) {
> -        return;
> -    }
> -
> -    ucc = USER_CREATABLE_GET_CLASS(uc);
>      if (ucc->complete) {
>          ucc->complete(uc, errp);
>      }
> @@ -89,7 +81,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
>          goto out;
>      }
>  
> -    user_creatable_complete(obj, &local_err);
> +    user_creatable_complete(USER_CREATABLE(obj), &local_err);
>      if (local_err) {
>          object_property_del(object_get_objects_root(),
>                              id, &error_abort);