Re: [Qemu-devel] [PATCH v2] numa: Clean up query-memdev error handling

[Michael S. Tsirkin]

On Mon, Nov 23, 2015 at 09:35:31AM +0100, Markus Armbruster wrote:
> qmp_query_memdev() has two error paths:
> 
> * When object_get_objects_root() returns null.  It never does, so
>   simply drop the useless error handling.
> 
> * When query_memdev() fails.  It leaks err then.  But any failure
>   there is actually a programming error.  Switch it to &error_abort,
>   and drop the useless error handling.
> 
> Messed up in commit 76b5d85 "qmp: add query-memdev".
> 
> Signed-off-by: Markus Armbruster <address@hidden>


Post 2.5 right?

> ---
>  numa.c | 59 ++++++++++-------------------------------------------------
>  1 file changed, 10 insertions(+), 49 deletions(-)
> 
> diff --git a/numa.c b/numa.c
> index fdfe294..1710946 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -517,7 +517,6 @@ static int query_memdev(Object *obj, void *opaque)
>  {
>      MemdevList **list = opaque;
>      MemdevList *m = NULL;
> -    Error *err = NULL;
>  
>      if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
>          m = g_malloc0(sizeof(*m));
> @@ -525,72 +524,34 @@ static int query_memdev(Object *obj, void *opaque)
>          m->value = g_malloc0(sizeof(*m->value));
>  
>          m->value->size = object_property_get_int(obj, "size",
> -                                                 &err);
> -        if (err) {
> -            goto error;
> -        }
> -
> +                                                 &error_abort);
>          m->value->merge = object_property_get_bool(obj, "merge",
> -                                                   &err);
> -        if (err) {
> -            goto error;
> -        }
> -
> +                                                   &error_abort);
>          m->value->dump = object_property_get_bool(obj, "dump",
> -                                                  &err);
> -        if (err) {
> -            goto error;
> -        }
> -
> +                                                  &error_abort);
>          m->value->prealloc = object_property_get_bool(obj,
> -                                                      "prealloc", &err);
> -        if (err) {
> -            goto error;
> -        }
> -
> +                                                      "prealloc",
> +                                                      &error_abort);
>          m->value->policy = object_property_get_enum(obj,
>                                                      "policy",
>                                                      "HostMemPolicy",
> -                                                    &err);
> -        if (err) {
> -            goto error;
> -        }
> -
> +                                                    &error_abort);
>          object_property_get_uint16List(obj, "host-nodes",
> -                                       &m->value->host_nodes, &err);
> -        if (err) {
> -            goto error;
> -        }
> +                                       &m->value->host_nodes,
> +                                       &error_abort);
>  
>          m->next = *list;
>          *list = m;
>      }
>  
>      return 0;
> -error:
> -    g_free(m->value);
> -    g_free(m);
> -
> -    return -1;
>  }
>  
>  MemdevList *qmp_query_memdev(Error **errp)
>  {
> -    Object *obj;
> +    Object *obj = object_get_objects_root();
>      MemdevList *list = NULL;
>  
> -    obj = object_get_objects_root();
> -    if (obj == NULL) {
> -        return NULL;
> -    }
> -
> -    if (object_child_foreach(obj, query_memdev, &list) != 0) {
> -        goto error;
> -    }
> -
> +    object_child_foreach(obj, query_memdev, &list);
>      return list;
> -
> -error:
> -    qapi_free_MemdevList(list);
> -    return NULL;
>  }
> -- 
> 2.4.3