[libvirt] [PATCH 3/6] rpc: Plug memory leak on virNetClientSendInternal() error path

Alex Jia ajia at redhat.com
Wed Nov 30 06:40:51 UTC 2011


On 11/30/2011 02:26 PM, Wen Congyang wrote:
> At 11/30/2011 01:57 PM, ajia at redhat.com Write:
>> From: Alex Jia<ajia at redhat.com>
>>
>> Detected by Coverity. Leak introduced in commit 673adba.
>>
>> Signed-off-by: Alex Jia<ajia at redhat.com>
>> ---
>>   src/rpc/virnetclient.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
>> index a738129..9ed03a5 100644
>> --- a/src/rpc/virnetclient.c
>> +++ b/src/rpc/virnetclient.c
>> @@ -1740,6 +1740,7 @@ cleanup:
>>
>>   unlock:
>>       virNetClientUnlock(client);
>> +    VIR_FREE(call);
> According to the comment:
> /* If partially sent, then the call is still on the dispatch queue */
>
> So I think we should not free call if it is still on the queue.
Yeah, it's a inappropriate place, in addition, in 'cleanup' label, if 
ret !=1, also will free 'all', then 'unlock' label still do this, the 
following should be a right place:

1708     if (!client->sock || client->wantClose) {
1709         virNetError(VIR_ERR_INTERNAL_ERROR, "%s",
1710                     _("client socket is closed"));
*1711       VIR_FREE(call);*
1712         goto unlock;
1713     }

Thanks,
Alex
> Thanks
> Wen Congyang
>
>>       return ret;
>>   }
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111130/01a8b0d3/attachment-0001.htm>


More information about the libvir-list mailing list