Re: [PATCH] admin_server: fix memory leak

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24.03.2016 14:02, Pavel Hrdina wrote:
> On Thu, Mar 24, 2016 at 01:34:04PM +0100, Erik Skultety wrote:
>> On 24/03/16 10:52, Pavel Hrdina wrote:
>>> If caller of adminConnectListServers() pass NULL instead of servers we
>>> need to free the list we've received from virNetDaemonGetServers().
>>>
>>
>> Good catch.
>>
>>> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
>>> ---
>>>  daemon/admin_server.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/daemon/admin_server.c b/daemon/admin_server.c
>>> index 1d16bc9..2f26488 100644
>>> --- a/daemon/admin_server.c
>>> +++ b/daemon/admin_server.c
>>> @@ -52,6 +52,8 @@ adminConnectListServers(virNetDaemonPtr dmn,
>>>      if (servers) {
>>>          *servers = srvs;
>>>          srvs = NULL;
>>> +    } else {
>>> +        virObjectListFreeCount(srvs, ret);
>>>      }
>>
>> No need to do it here ^^ actually, you can safely 'free' it in cleanup.
>> "srvs = NULL" guarantees that even if list was requested and non-null,
>> calling free in cleanup section will finish without any harm.
> 
> It was my intention to make it clear that only in this case we would free the
> srvs, I know, that it's safe to pass NULL to that function.
> 
>>
>>>   cleanup:
>>>      return ret;
>>>
>>
>> ACK with that adjustment.
> 
> Ok, I'll move it to cleanup :) thanks.

Oh, you forgot to guard it with check for ret > 0. Otherwise a terrible
thing will happen.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]