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

[Fabiano Rosas]

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

> ... but that IMHO doesn't really hurt, since the memory is not really 
> leaked, i.e. the memory usage won't increase during runtime here. So I fail 
> to see which problem you're really trying to solve here, could you please 
> elaborate?

You're right, its harmless. We could just not bother with it.