[libvirt] [PATCH] virsh.c: avoid leak on OOM error path

[Jim Meyering]

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.