Re: [libvirt] [PATCH 1/2] xenconfig: Properly check retval of virDomainGraphicsListenSetAddress

On 10/02/16 12:02, John Ferlan wrote: > > > On 02/10/2016 05:37 AM, Michal Privoznik wrote: >> The function, like others in our code, returns zero on success >> and a negative value on error. However, there are two places in >> xenconfig source code where we check for non-zero value. While >> the function can't currently return a positive value, those >> checks look okay, but does not really follow our style. >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> src/xenconfig/xen_sxpr.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> > > I have patches on list which are similar, but I've been requested to > change. In particular, this change leads to a memory leak. as the vnc > needs to be deleted. > > John Hmm, I can't see any memory leak caused by this particular patch, could you please point it out for me? The only thing that could possibly leak by an error within virDomainGraphicsListenAddress, is graphics and that pointer is properly freed at the end. Erik

>> diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c >> index 252a48b..fdfec2b 100644 >> --- a/src/xenconfig/xen_sxpr.c >> +++ b/src/xenconfig/xen_sxpr.c >> @@ -868,7 +868,7 @@ xenParseSxprGraphicsOld(virDomainDefPtr def, >> graphics->data.vnc.port = port; >> >> if (listenAddr && >> - virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, -1, true)) >> + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, -1, true) < 0) >> goto error; >> >> if (VIR_STRDUP(graphics->data.vnc.auth.passwd, vncPasswd) < 0) >> @@ -987,7 +987,7 @@ xenParseSxprGraphicsNew(virDomainDefPtr def, >> graphics->data.vnc.port = port; >> >> if (listenAddr && >> - virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, -1, true)) >> + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, -1, true) < 0) >> goto error; >> >> if (VIR_STRDUP(graphics->data.vnc.auth.passwd, vncPasswd) < 0) >> > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list >