On 13/07/2016 06:18, Xiao Guangrong wrote:
>
> Return MAX_NODES under this case to fix this bug
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
> ---
> backends/hostmem.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 6e28be1..8dede4d 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -64,6 +64,14 @@ out:
> error_propagate(errp, local_err);
> }
>
> +static uint16List **host_memory_append_node(uint16List **node,
> + unsigned long value)
> +{
> + *node = g_malloc0(sizeof(**node));
> + (*node)->value = value;
> + return &(*node)->next;
> +}
> +
> static void
> host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> @@ -74,25 +82,23 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
> unsigned long value;
>
> value = find_first_bit(backend->host_nodes, MAX_NODES);
> +
> + node = host_memory_append_node(node, value);
> +
> if (value == MAX_NODES) {
> - return;
> + goto out;
> }
>
> - *node = g_malloc0(sizeof(**node));
> - (*node)->value = value;
> - node = &(*node)->next;
> -
> do {
> value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1);
> if (value == MAX_NODES) {
> break;
> }
>
> - *node = g_malloc0(sizeof(**node));
> - (*node)->value = value;
> - node = &(*node)->next;
> + node = host_memory_append_node(node, value);
> } while (true);
>
> +out:
> visit_type_uint16List(v, name, &host_nodes, errp);
This function is leaking host_nodes, so you need a
qapi_free_uint16List(head);
here (and saving the head pointer on the first call to
host_memory_append_node). The bug is preexisting.
I'm curious about one thing. Eric/Markus, it would be nice to open code
the visit of the list with
visit_start_list(v, name, NULL, 0, &err);
if (err) {
goto out;
}
...
visit_type_uint16(v, name, &value, &err);
visit_next_list(v, NULL, 0);
...
visit_end_list(v, NULL);
We know here that on the other side there is an output visitor.
However, it doesn't work because visit_next_list asserts that tail ==
NULL. Would it be easy to support this idiom, and would it make sense
to extend it to other kinds of visitor?
Thanks,
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
