[libvirt] [PATCH] network: avoid memory leak on cleanup

[Alex Jia]

On 07/15/2011 03:03 PM, Alex Jia wrote:
> On 07/15/2011 02:49 PM, Matthias Bolte wrote:
>> 2011/7/15<ajia at redhat.com>:
>>> * src/network/bridge_driver.c: Fix memory leak on cleanup section from
>>>   networkGetBridgeName function.
>>> ---
>>>   src/network/bridge_driver.c |    3 ++-
>>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>>> index 0a12bc0..59e780d 100644
>>> --- a/src/network/bridge_driver.c
>>> +++ b/src/network/bridge_driver.c
>>> @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) {
>>>   cleanup:
>>>      if (network)
>>>          virNetworkObjUnlock(network);
>>> -    return bridge;
>>> +    VIR_FREE(bridge);
>>> +    return NULL;
>>>   }
>>>
>>>   static int networkGetAutostart(virNetworkPtr net,
>> NACK. Now networkGetBridgeName returns NULL always, that's wrong.
>>
>> Why do you think that there is a leak?
>>
> Detected in valgrind run:
> ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26
> ==9020==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
> ==9020==    by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
> ==9020==    by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30)
> ==9020==    by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999)
> ==9020==    by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286)
> ==9020==    by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318)
> ==9020==    by 0x4CE7262: call (remote_driver.c:3925)
> ==9020==    by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384)
> ==9020==    by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503)
> ==9020==    by 0x40F654: cmdNetworkInfo (virsh.c:5015)
> ==9020==    by 0x410D02: vshCommandRun (virsh.c:12738)
> ==9020==    by 0x41F2D5: main (virsh.c:14084)
> ==9020==
> ==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26
> ==9020==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
> ==9020==    by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so)
> ==9020==    by 0x4CF508B: doRemoteOpen (remote_driver.c:364)
> ==9020==    by 0x4CF6E8F: remoteOpen (remote_driver.c:800)
> ==9020==    by 0x4CCB384: do_open (libvirt.c:1054)
> ==9020==    by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280)
> ==9020==    by 0x410BC0: vshReconnect (virsh.c:589)
> ==9020==    by 0x410DCF: vshCommandRun (virsh.c:12733)
> ==9020==    by 0x41F2D5: main (virsh.c:14084)
> ==9020==
> ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26
> ==9020==    at 0x4A04A28: calloc (vg_replace_malloc.c:467)
> ==9020==    by 0x4C686ED: virAlloc (memory.c:101)
> ==9020==    by 0x4C96870: virDomainEventStateNew (domain_event.c:578)
> ==9020==    by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658)
> ==9020==    by 0x4CF6E8F: remoteOpen (remote_driver.c:800)
> ==9020==    by 0x4CCB384: do_open (libvirt.c:1054)
> ==9020==    by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280)
> ==9020==    by 0x410BC0: vshReconnect (virsh.c:589)
> ==9020==    by 0x410DCF: vshCommandRun (virsh.c:12733)
> ==9020==    by 0x41F2D5: main (virsh.c:14084)
>
> Note: remote memory leak are a known bug:https://bugzilla.redhat.com/show_bug.cgi?id=690734,
>        so this patch only fix network memory leak.
>
> The following codes leads memory leak:
> 2448 static char *networkGetBridgeName(virNetworkPtr net) {
> 2449     struct network_driver *driver = net->conn->networkPrivateData;
> 2450     virNetworkObjPtr network;
> 2451*char *bridge = NULL;*
> ......
> 2470*bridge = strdup(network->def->bridge);*
>
> Cleanup section hasn't released allocation free, and the programming will return a NULL if only and only
> error condition happen, if everything is okay, 'bridge' will be returned, right?
>
>
> Regards,
> Alex
>
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Yeah, we need to return 'bridge' pointer variable, so shouldn't free it, 
I should fix other function memory leak.

Thanks,
Alex
Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110715/5f2bd559/attachment-0001.htm>