[libvirt] [PATCH] qemu: fix memory leak in qemuAgentGetFSInfo
Ján Tomko
jtomko at redhat.com
Tue Mar 10 14:37:26 UTC 2015
On Tue, Mar 10, 2015 at 10:46:45AM +0000, Daniel P. Berrange wrote:
> On Tue, Mar 10, 2015 at 11:38:52AM +0100, Pavel Hrdina wrote:
> > On Tue, Mar 10, 2015 at 10:32:04AM +0100, Ján Tomko wrote:
> > > On Tue, Mar 10, 2015 at 01:56:11PM +0800, Chen Fan wrote:
> > > > in virDomainFSInfoFree(), don't free the virDomainFSInfo data.
> > > >
> > > > ==10670== 80 bytes in 2 blocks are definitely lost in loss record 576 of 793
> > > > ==10670== at 0x4A06BC3: calloc (vg_replace_malloc.c:618)
> > > > ==10670== by 0x509DEBD: virAlloc (viralloc.c:144)
> > > > ==10670== by 0x19FBD558: qemuAgentGetFSInfo (qemu_agent.c:1837)
> > > > ==10670== by 0x1A03CF91: qemuDomainGetFSInfo (qemu_driver.c:19238)
> > > >
> > > > Signed-off-by: Chen Fan <chen.fan.fnst at cn.fujitsu.com>
> > > > ---
> > > > src/libvirt-domain.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > >
> > > This does fix the memory leak and makes the function behave like it's
> > > documented in virDomainGetFSInfo and virDomainFSInfoFree:
> > > http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetFSInfo
> > > http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSInfoFree
> > >
> > > But it changes the public API - if there are applications that already
> > > work around this function by freeing the info, this change would
> > > introduce a double free.
> >
> > Well, this is a good point that we might break somebody's application, but the
> > virDomainFSInfoFree is the only one *Free API that doesn't call free/unref on
> > passed argument. In the documentation the @info is an array and we specifically
> > says that calling virDomainFSInfoFree on each element and then just free the
> > @info is sufficient, but it isn't and there will be a memory leak. It changes
> > behavior of public API to correctly free the allocated memory as documentation
> > says. Let's hear another opinion, but I'm for fixing it.
>
> I agree, in the normal way of usage of this API, this will always be wrong.
> virsh is leaking this, python bindings are leaking this, perl bindings are
> leaking this. Only applications written in C have the opportunity to do a
> workaround, and most of our apps are not written in C. I think we have to
> fix this really.
Now pushed.
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150310/28605105/attachment-0001.sig>
More information about the libvir-list
mailing list