Li Qiang <liq...@gmail.com> writes:
> Markus Armbruster <arm...@redhat.com> 于2020年7月16日周四 下午1:59写道：
>>
>> Li Qiang <liq...@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.au...@redhat.com>
>> > Signed-off-by: Li Qiang <liq...@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 <arm...@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()?
