Re: [Qemu-devel] [PATCH] test-keyval: fix leaks

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  tests/test-keyval.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/test-keyval.c b/tests/test-keyval.c
> index ba19560a22..141ee5d0c4 100644
> --- a/tests/test-keyval.c
> +++ b/tests/test-keyval.c
> @@ -628,6 +628,7 @@ static void test_keyval_visit_alternate(void)
>      visit_type_AltNumStr(v, "a", &ans, &error_abort);
>      g_assert_cmpint(ans->type, ==, QTYPE_QSTRING);
>      g_assert_cmpstr(ans->u.s, ==, "1");
> +    qapi_free_AltNumStr(ans);
>      visit_type_AltNumInt(v, "a", &ani, &err);
>      error_free_or_abort(&err);
>      visit_end_struct(v, NULL);
> @@ -649,11 +650,14 @@ static void test_keyval_visit_any(void)
>      visit_type_any(v, "a", &any, &error_abort);

@any becomes a strong reference (qobject_input_type_any() increments the
reference count).

>      qlist = qobject_to_qlist(any);

Reference count unchanged.

>      g_assert(qlist);
> +    qobject_decref(any);

Uh, this is unnecessarily dirty: you relinquish the reference before
you're actually done with it.  Works only because there's *another*
reference hiding within @v.  Let's move this ...

>      qstr = qobject_to_qstring(qlist_pop(qlist));
>      g_assert_cmpstr(qstring_get_str(qstr), ==, "null");
> +    QDECREF(qstr);
>      qstr = qobject_to_qstring(qlist_pop(qlist));
>      g_assert_cmpstr(qstring_get_str(qstr), ==, "1");
>      g_assert(qlist_empty(qlist));
> +    QDECREF(qstr);

... here.  Okay to do that on commit?

>      visit_check_struct(v, &error_abort);
>      visit_end_struct(v, NULL);
>      visit_free(v);

With the reference counting cleaned up:
Reviewed-by: Markus Armbruster <address@hidden>