Re: [PATCH] tests/qtest: Plug memory leaks in qtest_get_machines

[Thomas Huth <thuth@redhat.com>]

On 24/01/2023 13.45, Fabiano Rosas wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 23/01/2023 22.22, Fabiano Rosas wrote:
>>> Thomas Huth <thuth@redhat.com> writes:
>>>
>>>> On 23/01/2023 14.32, Fabiano Rosas wrote:
>>>>> Thomas Huth <thuth@redhat.com> writes:
>>>>>
>>>>>> On 20/01/2023 20.44, Fabiano Rosas wrote:
>>>>>>> These leaks can be avoided:
>>>>>>>
>>>>>>>      759 bytes in 61 blocks are still reachable in loss record 56 of 60
>>>>>>>         at 0x4034744: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>>>>>         by 0x4A88518: g_malloc (in /usr/lib64/libglib-2.0.so.0.7000.5)
>>>>>>>         by 0x4AA313E: g_strdup (in /usr/lib64/libglib-2.0.so.0.7000.5)
>>>>>>>         by 0x12083E: qtest_get_machines (libqtest.c:1323)
>>>>>>>         by 0x12098C: qtest_cb_for_every_machine (libqtest.c:1348)
>>>>>>>         by 0x11556C: main (test-hmp.c:160)
>>>>>>>
>>>>>>>      992 bytes in 1 blocks are still reachable in loss record 57 of 60
>>>>>>>         at 0x4034744: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>>>>>         by 0x4A88518: g_malloc (in /usr/lib64/libglib-2.0.so.0.7000.5)
>>>>>>>         by 0x120725: qtest_get_machines (libqtest.c:1313)
>>>>>>>         by 0x12098C: qtest_cb_for_every_machine (libqtest.c:1348)
>>>>>>>         by 0x11556C: main (test-hmp.c:160)
>> ...
>>>> (Also, it's valgrind that you used, isn't it? - I wonder why it's
>>>> complaining here at all since the pointer to the memory should still be
>>>> valid at the end?)
>>>
>>> valgrind is complaining about the memory not being explicitly freed at
>>> any point. I guess "leak" was not the most precise term to use in the
>>> commit message.
>>
>> How did you run valgrind? For me, it does not really complain about the
>> non-freed memory here since it is still reachable. The only difference that
>> I see is in the summary. Without your patch:
>>
>>       still reachable: 27,471 bytes in 152 blocks
>>
>> with your patch:
>>
>>       still reachable: 25,713 bytes in 88 blocks
> 
> valgrind --leak-check=full --show-leak-kinds=all

Ok, so that "--show-leak-kinds=all" is the issue here. I think it does not 
make sense to go hunting for each and every "reachable" non-freed memory 
block - since they have not really been leaked during runtime. It's maybe 
better if you just use "--show-leak-kinds=definite,indirect,possible" instead.

  Thomas