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