Re: [libvirt] [PATCH] qemu: Avoid memleak on failure to format blockjobs

On 10/18/18 3:08 PM, Eric Blake wrote: > virXMLFormatElement() frees attrBuf on success, but not necessarily > on failure. Most other callers of this function take the time to > reset attrBuf afterwords, but qemuDomainObjPrivateXMLFOrmatBlockjobs() > was relying on it succeeding, and could thus result in a memory leak. > > Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; > --- >   src/qemu/qemu_domain.c | 6 +++++- >   1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index dd67be5e2a..ad7a6fe136 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -2232,11 +2232,15 @@ > qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, >   { >       virBuffer attrBuf = VIR_BUFFER_INITIALIZER; >       bool bj = qemuDomainHasBlockjob(vm, false); > +    ret = -1; I need to quit editing patches in my mailer. As written, this obviously doesn't compile; s/ret/int ret/ > >       virBufferAsprintf(&attrBuf, " active='%s'", >                         > virTristateBoolTypeToString(virTristateBoolFromBool(bj))); > > -    return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); > +    ret = virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); > + cleanup: and this label is unused. > +    virBufferFreeAndReset(&attrBuf); > +    return ret; >   } > >