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

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(a)redhat.com&gt; >> --- >> 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.