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

Re: [libvirt] [PATCH] admin_server: fix memory leak



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 redhat com>
>>> ---
>>>  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


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