[libvirt] [PATCH] virsh: avoid memory leak on cmdVolCreateAs
* tools/virsh.c: fix memory leak on cmdVolCreateAs function. * Detected in valgrind run: ==4746== ==4746== 48 (40 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 26 of 52 ==4746== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==4746== by 0x4C76E51: virAlloc (memory.c:101) ==4746== by 0x4CD9418: virGetStoragePool (datatypes.c:592) ==4746== by 0x4D21367: remoteStoragePoolLookupByName (remote_driver.c:4126) ==4746== by 0x4CE42B0: virStoragePoolLookupByName (libvirt.c:10232) ==4746== by 0x40C276: vshCommandOptPoolBy (virsh.c:13660) ==4746== by 0x40CA37: cmdVolCreateAs (virsh.c:8094) ==4746== by 0x412AF2: vshCommandRun (virsh.c:13770) ==4746== by 0x422F11: main (virsh.c:15127) ==4746== ==4746== 1,011 bytes in 1 blocks are definitely lost in loss record 45 of 52 ==4746== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==4746== by 0x4A06167: realloc (vg_replace_malloc.c:525) ==4746== by 0x4C76ECB: virReallocN (memory.c:161) ==4746== by 0x4C60319: virBufferGrow (buf.c:72) ==4746== by 0x4C606AA: virBufferAdd (buf.c:106) ==4746== by 0x40CB37: cmdVolCreateAs (virsh.c:8118) ==4746== by 0x412AF2: vshCommandRun (virsh.c:13770) ==4746== by 0x422F11: main (virsh.c:15127) ==4746== ==4746== LEAK SUMMARY: ==4746== definitely lost: 1,051 bytes in 2 blocks ==4746== indirectly lost: 8 bytes in 1 blocks ==4746== possibly lost: 0 bytes in 0 blocks ==4746== still reachable: 390,767 bytes in 1,373 blocks ==4746== suppressed: 0 bytes in 0 blocks * How to reproduce? % valgrind -v --leak-check=full virsh vol-create-as default foo.img 10M \ --allocation 0 --format qcow2 --backing-vol bar.img Notes: bar.img doesn't exist. Signed-off-by: Alex Jia <ajia@redhat.com> --- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index f9bcd2c..44c2f1c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8166,7 +8166,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) } if (snapVol == NULL) { vshError(ctl, _("failed to get vol '%s'"), snapshotStrVol); - return false; + goto cleanup; } char *snapshotStrVolPath; -- 1.7.1
On 09/01/2011 11:12 AM, Alex Jia wrote:
* tools/virsh.c: fix memory leak on cmdVolCreateAs function.
% valgrind -v --leak-check=full virsh vol-create-as default foo.img 10M \ --allocation 0 --format qcow2 --backing-vol bar.img
Notes: bar.img doesn't exist.
Signed-off-by: Alex Jia<ajia@redhat.com> --- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index f9bcd2c..44c2f1c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8166,7 +8166,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) } if (snapVol == NULL) { vshError(ctl, _("failed to get vol '%s'"), snapshotStrVol); - return false; + goto cleanup;
Incomplete. There were a couple other early returns in this function. I'm pushing with this squashed in: diff --git i/tools/virsh.c w/tools/virsh.c index 7453995..5c5343e 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -8211,7 +8211,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) char *snapshotStrVolPath; if ((snapshotStrVolPath = virStorageVolGetPath(snapVol)) == NULL) { virStorageVolFree(snapVol); - return false; + goto cleanup; } /* Create XML for the backing store */ @@ -8230,7 +8230,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) if (virBufferError(&buf)) { vshPrint(ctl, "%s", _("Failed to allocate XML buffer")); - return false; + goto cleanup; } xml = virBufferContentAndReset(&buf); vol = virStorageVolCreateXML(pool, xml, 0); -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 09/02/2011 02:41 AM, Eric Blake wrote:
On 09/01/2011 11:12 AM, Alex Jia wrote:
* tools/virsh.c: fix memory leak on cmdVolCreateAs function.
% valgrind -v --leak-check=full virsh vol-create-as default foo.img 10M \ --allocation 0 --format qcow2 --backing-vol bar.img
Notes: bar.img doesn't exist.
Signed-off-by: Alex Jia<ajia@redhat.com> --- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index f9bcd2c..44c2f1c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8166,7 +8166,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) } if (snapVol == NULL) { vshError(ctl, _("failed to get vol '%s'"), snapshotStrVol); - return false; + goto cleanup;
Incomplete. There were a couple other early returns in this function. I'm pushing with this squashed in:
diff --git i/tools/virsh.c w/tools/virsh.c index 7453995..5c5343e 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -8211,7 +8211,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) char *snapshotStrVolPath; if ((snapshotStrVolPath = virStorageVolGetPath(snapVol)) == NULL) { virStorageVolFree(snapVol); - return false; + goto cleanup; }
/* Create XML for the backing store */ @@ -8230,7 +8230,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
if (virBufferError(&buf)) { vshPrint(ctl, "%s", _("Failed to allocate XML buffer")); - return false; + goto cleanup; } xml = virBufferContentAndReset(&buf); vol = virStorageVolCreateXML(pool, xml, 0);
Yeah, should release memory before returning. Thanks, Alex
participants (3)
-
ajia＠redhat.com -
Alex Jia
-
Eric Blake