Jiri Denemark wrote:
>> > No one really cares if we leak memory while dying, but who knows...
>> > freeing that buffer may let us go down more gracefully.
>> >
>> > FYI, the leak is triggered when virFileReadAll succeeds
>> > (it allocates "buffer"), yet xmlNewDoc fails:
>> >
>> > if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
>> > return FALSE;
>> >
>> > doc = xmlNewDoc(NULL);
>> > if (doc == NULL)
>> > goto no_memory;
>>
>> The above is correct, but there's another leak in the same function,
>> so I've amended the patch to also free the "list" buffer.
>> "list" is allocated in the for-loop.
>> If on the 2nd or subsequent iteration of that loop we take
>> the "goto no_memory", we'd leak that buffer.
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index dd916f3..c8ae9f2 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -7139,6 +7139,8 @@ cleanup:
>> return ret;
>>
>> no_memory:
>> + VIR_FREE(list);
>> + VIR_FREE(buffer);
>> vshError(ctl, "%s", _("Out of memory"));
>> ret = FALSE;
>> return ret;
>
> Actually that's not enough and it also duplicates code as all the cleanup code
> is already there:
Indeed.
The only piece of cleanup: code that is not needed
on the current no_memory path is the freeing of "result".
Plugging more leaks and avoiding duplication.
Definite improvement. ACK.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list