* [Qemu-devel] [PATCH] test-keyval: fix leaks
@ 2017-04-10 14:21 Marc-André Lureau
2017-04-10 15:28 ` Eric Blake
2017-04-12 11:19 ` Markus Armbruster
0 siblings, 2 replies; 5+ messages in thread
From: Marc-André Lureau @ 2017-04-10 14:21 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
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);
qlist = qobject_to_qlist(any);
g_assert(qlist);
+ qobject_decref(any);
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);
visit_check_struct(v, &error_abort);
visit_end_struct(v, NULL);
visit_free(v);
--
2.12.0.191.gc5d8de91d
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] test-keyval: fix leaks
2017-04-10 14:21 [Qemu-devel] [PATCH] test-keyval: fix leaks Marc-André Lureau
@ 2017-04-10 15:28 ` Eric Blake
2017-04-12 11:19 ` Markus Armbruster
1 sibling, 0 replies; 5+ messages in thread
From: Eric Blake @ 2017-04-10 15:28 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 512 bytes --]
On 04/10/2017 09:21 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> tests/test-keyval.c | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
Leaks in the testsuite are not showstoppers, so I'm okay if we defer it
to 2.10 to minimize churn now that we are late in hard freeze.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] test-keyval: fix leaks
2017-04-10 14:21 [Qemu-devel] [PATCH] test-keyval: fix leaks Marc-André Lureau
2017-04-10 15:28 ` Eric Blake
@ 2017-04-12 11:19 ` Markus Armbruster
2017-04-12 14:33 ` Marc-André Lureau
1 sibling, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2017-04-12 11:19 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel
Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 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 <armbru@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] test-keyval: fix leaks
2017-04-12 11:19 ` Markus Armbruster
@ 2017-04-12 14:33 ` Marc-André Lureau
2017-04-12 14:56 ` Markus Armbruster
0 siblings, 1 reply; 5+ messages in thread
From: Marc-André Lureau @ 2017-04-12 14:33 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
Hi
On Wed, Apr 12, 2017 at 3:19 PM Markus Armbruster <armbru@redhat.com> wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > 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?
>
sure, makes sense
> > visit_check_struct(v, &error_abort);
> > visit_end_struct(v, NULL);
> > visit_free(v);
>
> With the reference counting cleaned up:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
>
thanks
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] test-keyval: fix leaks
2017-04-12 14:33 ` Marc-André Lureau
@ 2017-04-12 14:56 ` Markus Armbruster
0 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2017-04-12 14:56 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Wed, Apr 12, 2017 at 3:19 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> > 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?
>>
>
> sure, makes sense
>
>
>> > visit_check_struct(v, &error_abort);
>> > visit_end_struct(v, NULL);
>> > visit_free(v);
>>
>> With the reference counting cleaned up:
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>
>>
> thanks
Applied to qapi-next, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-12 14:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-10 14:21 [Qemu-devel] [PATCH] test-keyval: fix leaks Marc-André Lureau
2017-04-10 15:28 ` Eric Blake
2017-04-12 11:19 ` Markus Armbruster
2017-04-12 14:33 ` Marc-André Lureau
2017-04-12 14:56 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.