* [PATCH v2] tests: qmp-cmd-test: fix memory leak
@ 2020-07-15 15:41 Li Qiang
2020-07-16 5:59 ` Markus Armbruster
2020-07-16 7:56 ` Thomas Huth
0 siblings, 2 replies; 7+ messages in thread
From: Li Qiang @ 2020-07-15 15:41 UTC (permalink / raw)
To: armbru, thuth, lvivier, pbonzini, eric.auger; +Cc: Li Qiang, liq3ea, qemu-devel
Properly free each test response to avoid memory leak and separate
qtest_qmp() calls with spare lines, in a consistent manner.
Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add
qmp/object-add-failure-modes"
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Li Qiang <liq3ea@163.com>
---
Change sincve v1: add detailed commit message
tests/qtest/qmp-cmd-test.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index c68f99f659..f7b1aa7fdc 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -230,6 +230,8 @@ static void test_object_add_failure_modes(void)
" 'props': {'size': 1048576 } } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
" {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
" 'props': {'size': 1048576 } } }");
@@ -241,6 +243,7 @@ static void test_object_add_failure_modes(void)
" {'id': 'ram1' } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
/* attempt to create an object with a property of a wrong type */
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -249,17 +252,20 @@ static void test_object_add_failure_modes(void)
g_assert_nonnull(resp);
/* now do it right */
qmp_assert_error_class(resp, "GenericError");
+
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
" {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
" 'props': {'size': 1048576 } } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
/* delete ram1 object */
resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
" {'id': 'ram1' } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
/* attempt to create an object without the id */
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -267,18 +273,21 @@ static void test_object_add_failure_modes(void)
" 'props': {'size': 1048576 } } }");
g_assert_nonnull(resp);
qmp_assert_error_class(resp, "GenericError");
+
/* now do it right */
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
" {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
" 'props': {'size': 1048576 } } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
/* delete ram1 object */
resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
" {'id': 'ram1' } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
/* attempt to set a non existing property */
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -286,23 +295,27 @@ static void test_object_add_failure_modes(void)
" 'props': {'sized': 1048576 } } }");
g_assert_nonnull(resp);
qmp_assert_error_class(resp, "GenericError");
+
/* now do it right */
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
" {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
" 'props': {'size': 1048576 } } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
/* delete ram1 object without id */
resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
" {'ida': 'ram1' } }");
g_assert_nonnull(resp);
+ qobject_unref(resp);
/* delete ram1 object */
resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
" {'id': 'ram1' } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
/* delete ram1 object that does not exist anymore*/
resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] tests: qmp-cmd-test: fix memory leak 2020-07-15 15:41 [PATCH v2] tests: qmp-cmd-test: fix memory leak Li Qiang @ 2020-07-16 5:59 ` Markus Armbruster 2020-07-16 6:43 ` Li Qiang 2020-07-16 7:56 ` Thomas Huth 1 sibling, 1 reply; 7+ messages in thread From: Markus Armbruster @ 2020-07-16 5:59 UTC (permalink / raw) To: Li Qiang; +Cc: lvivier, thuth, liq3ea, qemu-devel, eric.auger, pbonzini Li Qiang <liq3ea@163.com> writes: > Properly free each test response to avoid memory leak and separate > qtest_qmp() calls with spare lines, in a consistent manner. > > Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add > qmp/object-add-failure-modes" The patch also fixes leaks introduced in 442b09b83d and 9fc719b869, actually. At least it should, but the patch appears to be incomplete. > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Signed-off-by: Li Qiang <liq3ea@163.com> > --- > Change sincve v1: add detailed commit message > > tests/qtest/qmp-cmd-test.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c > index c68f99f659..f7b1aa7fdc 100644 > --- a/tests/qtest/qmp-cmd-test.c > +++ b/tests/qtest/qmp-cmd-test.c > @@ -230,6 +230,8 @@ static void test_object_add_failure_modes(void) static void test_object_add_failure_modes(void) { QTestState *qts; QDict *resp; /* attempt to create an object without props */ qts = qtest_init(common_args); resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }"); g_assert_nonnull(resp); qmp_assert_error_class(resp, "GenericError"); Doesn't @resp leak here, too? /* attempt to create an object without qom-type */ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" " {'id': 'ram1' } }"); g_assert_nonnull(resp); qmp_assert_error_class(resp, "GenericError"); Likewise. /* attempt to delete an object that does not exist */ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" " {'id': 'ram1' } }"); g_assert_nonnull(resp); qmp_assert_error_class(resp, "GenericError"); Likewise. /* attempt to create 2 objects with duplicate id */ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > " 'props': {'size': 1048576 } } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > + > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > " 'props': {'size': 1048576 } } }"); g_assert_nonnull(resp); qmp_assert_error_class(resp, "GenericError"); Likewise. /* delete ram1 object */ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > @@ -241,6 +243,7 @@ static void test_object_add_failure_modes(void) > " {'id': 'ram1' } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > > /* attempt to create an object with a property of a wrong type */ > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," " 'props': {'size': '1048576' } } }"); > @@ -249,17 +252,20 @@ static void test_object_add_failure_modes(void) > g_assert_nonnull(resp); > /* now do it right */ > qmp_assert_error_class(resp, "GenericError"); Likewise. > + > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > " 'props': {'size': 1048576 } } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > > /* delete ram1 object */ > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > " {'id': 'ram1' } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > > /* attempt to create an object without the id */ > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" " {'qom-type': 'memory-backend-ram'," > @@ -267,18 +273,21 @@ static void test_object_add_failure_modes(void) > " 'props': {'size': 1048576 } } }"); > g_assert_nonnull(resp); > qmp_assert_error_class(resp, "GenericError"); Likewise. > + > /* now do it right */ > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > " 'props': {'size': 1048576 } } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > > /* delete ram1 object */ > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > " {'id': 'ram1' } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > > /* attempt to set a non existing property */ > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > @@ -286,23 +295,27 @@ static void test_object_add_failure_modes(void) > " 'props': {'sized': 1048576 } } }"); > g_assert_nonnull(resp); > qmp_assert_error_class(resp, "GenericError"); Likewise. > + > /* now do it right */ > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > " 'props': {'size': 1048576 } } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > > /* delete ram1 object without id */ > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > " {'ida': 'ram1' } }"); > g_assert_nonnull(resp); > + qobject_unref(resp); > > /* delete ram1 object */ > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > " {'id': 'ram1' } }"); > g_assert_nonnull(resp); > g_assert(qdict_haskey(resp, "return")); > + qobject_unref(resp); > > /* delete ram1 object that does not exist anymore*/ > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" " {'id': 'ram1' } }"); g_assert_nonnull(resp); qmp_assert_error_class(resp, "GenericError"); Likewise. qtest_quit(qts); } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tests: qmp-cmd-test: fix memory leak 2020-07-16 5:59 ` Markus Armbruster @ 2020-07-16 6:43 ` Li Qiang 2020-07-16 9:52 ` Markus Armbruster 0 siblings, 1 reply; 7+ messages in thread From: Li Qiang @ 2020-07-16 6:43 UTC (permalink / raw) To: Markus Armbruster Cc: Laurent Vivier, Thomas Huth, Li Qiang, Qemu Developers, Auger Eric, Paolo Bonzini Markus Armbruster <armbru@redhat.com> 于2020年7月16日周四 下午1:59写道： > > Li Qiang <liq3ea@163.com> writes: > > > Properly free each test response to avoid memory leak and separate > > qtest_qmp() calls with spare lines, in a consistent manner. > > > > Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add > > qmp/object-add-failure-modes" > > The patch also fixes leaks introduced in 442b09b83d and 9fc719b869, > actually. At least it should, but the patch appears to be incomplete. > > > > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Signed-off-by: Li Qiang <liq3ea@163.com> > > --- > > Change sincve v1: add detailed commit message > > > > tests/qtest/qmp-cmd-test.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c > > index c68f99f659..f7b1aa7fdc 100644 > > --- a/tests/qtest/qmp-cmd-test.c > > +++ b/tests/qtest/qmp-cmd-test.c > > @@ -230,6 +230,8 @@ static void test_object_add_failure_modes(void) > static void test_object_add_failure_modes(void) > { > QTestState *qts; > QDict *resp; > > /* attempt to create an object without props */ > qts = qtest_init(common_args); > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }"); > g_assert_nonnull(resp); > qmp_assert_error_class(resp, "GenericError"); > > Doesn't @resp leak here, too? No, qmp_assert_error_class will call qobject_unref(rsp) will so will not leak. In fact, I think this is a inconsistent for 'qtest_qmp'. I think we can apply this patch first and then change the 'qmp_assert_error_class' or/and others to free resp. And just let the caller of 'qtest_qmp' frees unref the rsp. What's your idea? Thanks, Li Qiang > > /* attempt to create an object without qom-type */ > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'id': 'ram1' } }"); > g_assert_nonnull(resp); > qmp_assert_error_class(resp, "GenericError"); > > Likewise. > > /* attempt to delete an object that does not exist */ > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > " {'id': 'ram1' } }"); > g_assert_nonnull(resp); > qmp_assert_error_class(resp, "GenericError"); > > Likewise. > > /* attempt to create 2 objects with duplicate id */ > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > > " 'props': {'size': 1048576 } } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > + > > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > > " 'props': {'size': 1048576 } } }"); > g_assert_nonnull(resp); > qmp_assert_error_class(resp, "GenericError"); > > Likewise. > > /* delete ram1 object */ > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > > @@ -241,6 +243,7 @@ static void test_object_add_failure_modes(void) > > " {'id': 'ram1' } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > > > /* attempt to create an object with a property of a wrong type */ > > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > " 'props': {'size': '1048576' } } }"); > > @@ -249,17 +252,20 @@ static void test_object_add_failure_modes(void) > > g_assert_nonnull(resp); > > /* now do it right */ > > qmp_assert_error_class(resp, "GenericError"); > > Likewise. > > > + > > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > > " 'props': {'size': 1048576 } } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > > > /* delete ram1 object */ > > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > > " {'id': 'ram1' } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > > > /* attempt to create an object without the id */ > > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram'," > > @@ -267,18 +273,21 @@ static void test_object_add_failure_modes(void) > > " 'props': {'size': 1048576 } } }"); > > g_assert_nonnull(resp); > > qmp_assert_error_class(resp, "GenericError"); > > Likewise. > > > + > > /* now do it right */ > > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > > " 'props': {'size': 1048576 } } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > > > /* delete ram1 object */ > > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > > " {'id': 'ram1' } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > > > /* attempt to set a non existing property */ > > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > > @@ -286,23 +295,27 @@ static void test_object_add_failure_modes(void) > > " 'props': {'sized': 1048576 } } }"); > > g_assert_nonnull(resp); > > qmp_assert_error_class(resp, "GenericError"); > > Likewise. > > > + > > /* now do it right */ > > resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > > " {'qom-type': 'memory-backend-ram', 'id': 'ram1'," > > " 'props': {'size': 1048576 } } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > > > /* delete ram1 object without id */ > > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > > " {'ida': 'ram1' } }"); > > g_assert_nonnull(resp); > > + qobject_unref(resp); > > > > /* delete ram1 object */ > > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > > " {'id': 'ram1' } }"); > > g_assert_nonnull(resp); > > g_assert(qdict_haskey(resp, "return")); > > + qobject_unref(resp); > > > > /* delete ram1 object that does not exist anymore*/ > > resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':" > " {'id': 'ram1' } }"); > g_assert_nonnull(resp); > qmp_assert_error_class(resp, "GenericError"); > > Likewise. > > qtest_quit(qts); > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tests: qmp-cmd-test: fix memory leak 2020-07-16 6:43 ` Li Qiang @ 2020-07-16 9:52 ` Markus Armbruster 2020-07-16 10:33 ` Li Qiang 0 siblings, 1 reply; 7+ messages in thread From: Markus Armbruster @ 2020-07-16 9:52 UTC (permalink / raw) To: Li Qiang Cc: Laurent Vivier, Thomas Huth, Li Qiang, Qemu Developers, Auger Eric, Paolo Bonzini Li Qiang <liq3ea@gmail.com> writes: > Markus Armbruster <armbru@redhat.com> 于2020年7月16日周四 下午1:59写道： >> >> Li Qiang <liq3ea@163.com> writes: >> >> > Properly free each test response to avoid memory leak and separate >> > qtest_qmp() calls with spare lines, in a consistent manner. >> > >> > Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add >> > qmp/object-add-failure-modes" >> >> The patch also fixes leaks introduced in 442b09b83d and 9fc719b869, >> actually. At least it should, but the patch appears to be incomplete. 442b09b83d was fine, actually. 9fc719b869 wasn't, and your second patch hunk fixes it. Please add a "Fixes: 9fc719b869' line to the commit message. >> > >> > Reviewed-by: Eric Auger <eric.auger@redhat.com> >> > Signed-off-by: Li Qiang <liq3ea@163.com> >> > --- >> > Change sincve v1: add detailed commit message >> > >> > tests/qtest/qmp-cmd-test.c | 13 +++++++++++++ >> > 1 file changed, 13 insertions(+) >> > >> > diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c >> > index c68f99f659..f7b1aa7fdc 100644 >> > --- a/tests/qtest/qmp-cmd-test.c >> > +++ b/tests/qtest/qmp-cmd-test.c >> > @@ -230,6 +230,8 @@ static void test_object_add_failure_modes(void) >> static void test_object_add_failure_modes(void) >> { >> QTestState *qts; >> QDict *resp; >> >> /* attempt to create an object without props */ >> qts = qtest_init(common_args); >> resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" >> " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }"); >> g_assert_nonnull(resp); >> qmp_assert_error_class(resp, "GenericError"); >> >> Doesn't @resp leak here, too? > > No, qmp_assert_error_class will call qobject_unref(rsp) will so will not leak. You're right. With the additional Fixes: Reviewed-by: Markus Armbruster <armbru@redhat.com> > In fact, I think this is a inconsistent for 'qtest_qmp'. > I think we can apply this patch first and then change the > 'qmp_assert_error_class' or/and others > to free resp. And just let the caller of 'qtest_qmp' frees unref the rsp. Do you mean "not to free @resp"? > What's your idea? Rename it to qmp_expect_error_and_unref()? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tests: qmp-cmd-test: fix memory leak 2020-07-16 9:52 ` Markus Armbruster @ 2020-07-16 10:33 ` Li Qiang 2020-07-16 10:47 ` Thomas Huth 0 siblings, 1 reply; 7+ messages in thread From: Li Qiang @ 2020-07-16 10:33 UTC (permalink / raw) To: Markus Armbruster Cc: Laurent Vivier, Thomas Huth, Li Qiang, Qemu Developers, Auger Eric, Paolo Bonzini Markus Armbruster <armbru@redhat.com> 于2020年7月16日周四 下午5:52写道： > > Li Qiang <liq3ea@gmail.com> writes: > > > Markus Armbruster <armbru@redhat.com> 于2020年7月16日周四 下午1:59写道： > >> > >> Li Qiang <liq3ea@163.com> writes: > >> > >> > Properly free each test response to avoid memory leak and separate > >> > qtest_qmp() calls with spare lines, in a consistent manner. > >> > > >> > Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add > >> > qmp/object-add-failure-modes" > >> > >> The patch also fixes leaks introduced in 442b09b83d and 9fc719b869, > >> actually. At least it should, but the patch appears to be incomplete. > > 442b09b83d was fine, actually. > > 9fc719b869 wasn't, and your second patch hunk fixes it. Please add a > "Fixes: 9fc719b869' line to the commit message. Hi Thomas, Could you do this minor adjustment? Add also add Markus's r-b tag. > > >> > > >> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > >> > Signed-off-by: Li Qiang <liq3ea@163.com> > >> > --- > >> > Change sincve v1: add detailed commit message > >> > > >> > tests/qtest/qmp-cmd-test.c | 13 +++++++++++++ > >> > 1 file changed, 13 insertions(+) > >> > > >> > diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c > >> > index c68f99f659..f7b1aa7fdc 100644 > >> > --- a/tests/qtest/qmp-cmd-test.c > >> > +++ b/tests/qtest/qmp-cmd-test.c > >> > @@ -230,6 +230,8 @@ static void test_object_add_failure_modes(void) > >> static void test_object_add_failure_modes(void) > >> { > >> QTestState *qts; > >> QDict *resp; > >> > >> /* attempt to create an object without props */ > >> qts = qtest_init(common_args); > >> resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':" > >> " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }"); > >> g_assert_nonnull(resp); > >> qmp_assert_error_class(resp, "GenericError"); > >> > >> Doesn't @resp leak here, too? > > > > No, qmp_assert_error_class will call qobject_unref(rsp) will so will not leak. > > You're right. > > With the additional Fixes: > Reviewed-by: Markus Armbruster <armbru@redhat.com> > > > In fact, I think this is a inconsistent for 'qtest_qmp'. > > I think we can apply this patch first and then change the > > 'qmp_assert_error_class' or/and others > > to free resp. And just let the caller of 'qtest_qmp' frees unref the rsp. > > Do you mean "not to free @resp"? Yes, the 'qmp_assert_error_class' not free @resp. > > > What's your idea? > > Rename it to qmp_expect_error_and_unref()? In fact I prefer qmp_assert_error_class has no reason unref @resp. So the user of ''qtest_qmp' will has a unify processing of the @resp. resp = qtest_qmp() use resp free resp. If we just rename to 'qmp_expect_error_and_unref'. The user still should remember this. If not they will have UAF or mem leak issue. Thanks, Li Qiang > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tests: qmp-cmd-test: fix memory leak 2020-07-16 10:33 ` Li Qiang @ 2020-07-16 10:47 ` Thomas Huth 0 siblings, 0 replies; 7+ messages in thread From: Thomas Huth @ 2020-07-16 10:47 UTC (permalink / raw) To: Li Qiang, Markus Armbruster Cc: Laurent Vivier, Auger Eric, Li Qiang, Qemu Developers, Paolo Bonzini On 16/07/2020 12.33, Li Qiang wrote: > Markus Armbruster <armbru@redhat.com> 于2020年7月16日周四 下午5:52写道： >> >> Li Qiang <liq3ea@gmail.com> writes: >> >>> Markus Armbruster <armbru@redhat.com> 于2020年7月16日周四 下午1:59写道： >>>> >>>> Li Qiang <liq3ea@163.com> writes: >>>> >>>>> Properly free each test response to avoid memory leak and separate >>>>> qtest_qmp() calls with spare lines, in a consistent manner. >>>>> >>>>> Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add >>>>> qmp/object-add-failure-modes" >>>> >>>> The patch also fixes leaks introduced in 442b09b83d and 9fc719b869, >>>> actually. At least it should, but the patch appears to be incomplete. >> >> 442b09b83d was fine, actually. >> >> 9fc719b869 wasn't, and your second patch hunk fixes it. Please add a >> "Fixes: 9fc719b869' line to the commit message. > > Hi Thomas, > > Could you do this minor adjustment? > Add also add Markus's r-b tag. Sure, I'll add it! Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tests: qmp-cmd-test: fix memory leak 2020-07-15 15:41 [PATCH v2] tests: qmp-cmd-test: fix memory leak Li Qiang 2020-07-16 5:59 ` Markus Armbruster @ 2020-07-16 7:56 ` Thomas Huth 1 sibling, 0 replies; 7+ messages in thread From: Thomas Huth @ 2020-07-16 7:56 UTC (permalink / raw) To: Li Qiang, armbru, lvivier, pbonzini, eric.auger; +Cc: liq3ea, qemu-devel On 15/07/2020 17.41, Li Qiang wrote: > Properly free each test response to avoid memory leak and separate > qtest_qmp() calls with spare lines, in a consistent manner. > > Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add > qmp/object-add-failure-modes" > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Signed-off-by: Li Qiang <liq3ea@163.com> > --- > Change sincve v1: add detailed commit message > > tests/qtest/qmp-cmd-test.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) Thanks, queued to https://gitlab.com/huth/qemu/-/commits/qtest-next/ Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-16 10:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-15 15:41 [PATCH v2] tests: qmp-cmd-test: fix memory leak Li Qiang 2020-07-16 5:59 ` Markus Armbruster 2020-07-16 6:43 ` Li Qiang 2020-07-16 9:52 ` Markus Armbruster 2020-07-16 10:33 ` Li Qiang 2020-07-16 10:47 ` Thomas Huth 2020-07-16 7:56 ` Thomas Huth
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.