[libvirt] [PATCH] virsh: fix memory leak in cmdVolPath code

[Alex Jia]

On 07/28/2011 11:06 AM, Osier Yang wrote:
> 于 2011年07月27日 11:48, Osier Yang 写道:
>> 于 2011年07月27日 11:33, Alex Jia 写道:
>>> * tools/virsh.c: avoid memory leak in cmdVolPath.
>>>
>>> * how to reproduce?
>>>
>>>    % dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
>>>    % virsh pool-refresh default
>>>    % valgrind -v --leak-check=full virsh vol-path --vol
>>>       /var/lib/libvirt/images/foo.img
>>>
>>> * actual results:
>>>
>>> Detected in valgrind run:
>>>
>>> ==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 
>>> of 22
>>> ==16436==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
>>> ==16436==    by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
>>> ==16436==    by 0x3DF8CD770D: xdr_remote_nonnull_string 
>>> (remote_protocol.c:30)
>>> ==16436==    by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
>>> (remote_protocol.c:2952)
>>> ==16436==    by 0x3DF8CDF161: virNetMessageDecodePayload 
>>> (virnetmessage.c:286)
>>> ==16436==    by 0x3DF8CDE9E5: virNetClientProgramCall
>>> (virnetclientprogram.c:318)
>>> ==16436==    by 0x3DF8CC28A2: call (remote_driver.c:3929)
>>> ==16436==    by 0x3DF8CC8412: remoteStorageVolGetPath
>>> (remote_client_bodies.h:5219)
>>> ==16436==    by 0x3DF8C9BF14: virStorageVolGetPath (libvirt.c:11389)
>>> ==16436==    by 0x418ECA: cmdVolPath (virsh.c:8754)
>>> ==16436==    by 0x410CC2: vshCommandRun (virsh.c:12758)
>>> ==16436==    by 0x41F286: main (virsh.c:14110)
>>> ==16436==
>>> ==16436== LEAK SUMMARY:
>>> ==16436==    definitely lost: 32 bytes in 1 blocks
>>>
>>>
>>> Signed-off-by: Alex Jia<ajia at redhat.com>
>>> ---
>>>   tools/virsh.c |    9 ++++++++-
>>>   1 files changed, 8 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>> index 841df61..d194a8b 100644
>>> --- a/tools/virsh.c
>>> +++ b/tools/virsh.c
>>> @@ -9326,6 +9326,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
>>>   {
>>>       virStorageVolPtr vol;
>>>       const char *name = NULL;
>>> +    char * StorageVolPath;
>>>
>>>       if (!vshConnectionUsability(ctl, ctl->conn))
>>>           return false;
>>> @@ -9334,7 +9335,13 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
>>>           return false;
>>>       }
>>>
>>> -    vshPrint(ctl, "%s\n", virStorageVolGetPath(vol));
>>> +    if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
>>> +        virStorageVolFree(vol);
>>> +        return false;
>>> +    }
>>> +
>>> +    vshPrint(ctl, "%s\n", StorageVolPath);
>>> +    VIR_FREE(StorageVolPath);
>>>       virStorageVolFree(vol);
>>>       return true;
>>>   }
>>
>> ACK, but it will be better if docs of virStorageVolGetPath in 
>> src/libvirt.c
>> can be updated, (to tell the returned path must be freed by caller).
>>
>> Osier
>>
>> -- 
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
> Pushed with comments of virStorageVolGetPath updated. Thanks
>
> Osier
Osier, thanks for your push and update.

BTW, it seems other comments of function are also missing this 
introduction for whether caller should
free this.

Regards,
Alex