| Message ID | 20170410142112.11550-1-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
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.
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>
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 <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!
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);
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/test-keyval.c | 4 ++++ 1 file changed, 4 insertions(+)