Re: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects

On 01/24/2018 05:12 AM, Marc Hartmayer wrote: > On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com&gt; wrote: >> Once virNetServerProgramNew returns, the program objects have >> either been added to the @srv or @srvAdm list increasing the >> refcnt or there was an error. In either case, we can lower the >> refcnt from virNetServerProgramNew and set the object to NULL >> since it won't be used any more. >> >> The @srv or @srvAdm dispose function (virNetServerDispose) will >> handle the Unref of each object during the virHashFree when the >> object is removed from the hash table. >> >> Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> daemon/libvirtd.c | 28 ++++++++++++++++++++-------- >> 1 file changed, 20 insertions(+), 8 deletions(-) >> >> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c >> index 0afd1dd82..b47f875d9 100644 >> --- a/daemon/libvirtd.c >> +++ b/daemon/libvirtd.c >> @@ -1352,7 +1352,11 @@ int main(int argc, char **argv) { >> ret = VIR_DAEMON_ERR_INIT; >> goto cleanup; >> } >> - if (virNetServerAddProgram(srv, remoteProgram) < 0) { >> + >> + rc = virNetServerAddProgram(srv, remoteProgram); >> + virObjectUnref(remoteProgram); >> + remoteProgram = NULL; > > > I’ve just looked at the code and all those variables are a global > variables… > > libvirtd.c: > virNetServerProgramPtr remoteProgram = NULL; > virNetServerProgramPtr adminProgram = NULL; > virNetServerProgramPtr qemuProgram = NULL; > virNetServerProgramPtr lxcProgram = NULL; > > So this is not a good idea to set them to NULL… As this results in a > segmentation fault Oh right... There really was no reason to set them to NULL since the only time they're used is in the context they are used. If the AddProgram was successful, then there would be 2 refs anyway. They're not used later in the code, so I'll remove the = NULL after the Unref.

John >