[libvirt] [PATCH] virsh: plug memory leak on cmdBlkdeviotune() sucessful path
From: Alex Jia <ajia@redhat.com> Detected by valgrind. Leak introduced in commit e9bd9a0: * tools/virsh.c: fix memory leak on cmdBlkdeviotune. * how to reproduce? % valgrind -v --leak-check=full virsh blkdeviotune <domain name> <block device> * actual valgrind result: ==12759== 576 bytes in 1 blocks are definitely lost in loss record 18 of 29 ==12759== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==12759== by 0x42134E: _vshCalloc.clone.2 (virsh.c:422) ==12759== by 0x4217CB: cmdBlkdeviotune (virsh.c:6364) ==12759== by 0x4136A2: vshCommandRun (virsh.c:16363) ==12759== by 0x4253FB: main (virsh.c:17865) ==12759== ==12759== LEAK SUMMARY: ==12759== definitely lost: 576 bytes in 1 blocks ==12759== indirectly lost: 0 bytes in 0 blocks ==12759== possibly lost: 0 bytes in 0 blocks ==12759== still reachable: 126,964 bytes in 1,342 blocks ==12759== suppressed: 0 bytes in 0 blocks Signed-off-by: Alex Jia <ajia@redhat.com> --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index a51478f..e6e4f8b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6400,8 +6400,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) } } - virDomainFree(dom); - return true; + ret = true; + goto cleanup; } else { /* Set the block I/O throttle, match by opt since parameters can be 0 */ params = vshCalloc(ctl, nparams, sizeof(*params)); -- 1.7.1
From: Alex Jia <ajia@redhat.com> Detected by valgrind. Leak introduced in commit dc675f3: * tools/virsh.c: fix memory leak on cmdDomIfGetLink. * how to reproduce? % valgrind -v --leak-check=full virsh domif-getlink <domain name> 0 * actual valgrind result: ==13102== 18 bytes in 1 blocks are definitely lost in loss record 9 of 47 ==13102== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==13102== by 0x322A6A67DD: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6) ==13102== by 0x414892: cmdDomIfGetLink (virsh.c:1538) ==13102== by 0x4136A2: vshCommandRun (virsh.c:16363) ==13102== by 0x4253FB: main (virsh.c:17865) ==13102== ==13102== LEAK SUMMARY: ==13102== definitely lost: 18 bytes in 1 blocks ==13102== indirectly lost: 0 bytes in 0 blocks ==13102== possibly lost: 0 bytes in 0 blocks ==13102== still reachable: 127,888 bytes in 1,361 blocks ==13102== suppressed: 0 bytes in 0 blocks 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 e6e4f8b..276e1cc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1538,7 +1538,6 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) mac = virXMLPropString(cur, "address"); if (STRCASEEQ(mac, iface)){ - VIR_FREE(mac); goto hit; } } @@ -1574,6 +1573,7 @@ cleanup: if (dom) virDomainFree(dom); + VIR_FREE(mac); return ret; } -- 1.7.1
On 2011年12月08日 14:09, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
Detected by valgrind. Leak introduced in commit dc675f3:
* tools/virsh.c: fix memory leak on cmdDomIfGetLink.
* how to reproduce? % valgrind -v --leak-check=full virsh domif-getlink<domain name> 0
* actual valgrind result:
==13102== 18 bytes in 1 blocks are definitely lost in loss record 9 of 47 ==13102== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==13102== by 0x322A6A67DD: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6) ==13102== by 0x414892: cmdDomIfGetLink (virsh.c:1538) ==13102== by 0x4136A2: vshCommandRun (virsh.c:16363) ==13102== by 0x4253FB: main (virsh.c:17865) ==13102== ==13102== LEAK SUMMARY: ==13102== definitely lost: 18 bytes in 1 blocks ==13102== indirectly lost: 0 bytes in 0 blocks ==13102== possibly lost: 0 bytes in 0 blocks ==13102== still reachable: 127,888 bytes in 1,361 blocks ==13102== suppressed: 0 bytes in 0 blocks
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 e6e4f8b..276e1cc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1538,7 +1538,6 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) mac = virXMLPropString(cur, "address");
if (STRCASEEQ(mac, iface)){ - VIR_FREE(mac); goto hit; } } @@ -1574,6 +1573,7 @@ cleanup: if (dom) virDomainFree(dom);
+ VIR_FREE(mac); return ret; }
ACK. Osier
On 12/07/2011 11:22 PM, Osier Yang wrote:
On 2011年12月08日 14:09, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
Detected by valgrind. Leak introduced in commit dc675f3:
* tools/virsh.c: fix memory leak on cmdDomIfGetLink.
ACK.
Not so fast. mac is allocated in a loop, so it must be freed in a loop. I squashed in before I pushed: diff --git i/tools/virsh.c w/tools/virsh.c index 276e1cc..4262d60 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -1538,8 +1538,10 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) mac = virXMLPropString(cur, "address"); if (STRCASEEQ(mac, iface)){ + VIR_FREE(mac); goto hit; } + VIR_FREE(mac); } cur = cur->next; } @@ -1573,7 +1575,6 @@ cleanup: if (dom) virDomainFree(dom); - VIR_FREE(mac); return ret; } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Oh, It's my fault :(, thanks Eric and Osier. Regards, Alex ----- Original Message ----- From: "Eric Blake" <eblake@redhat.com> To: "Osier Yang" <jyang@redhat.com> Cc: ajia@redhat.com, libvir-list@redhat.com Sent: Friday, December 9, 2011 7:47:47 AM Subject: Re: [libvirt] [PATCH] virsh: plug memory leak on cmdDomIfGetLink() sucessful path On 12/07/2011 11:22 PM, Osier Yang wrote:
On 2011年12月08日 14:09, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
Detected by valgrind. Leak introduced in commit dc675f3:
* tools/virsh.c: fix memory leak on cmdDomIfGetLink.
ACK.
Not so fast. mac is allocated in a loop, so it must be freed in a loop. I squashed in before I pushed: diff --git i/tools/virsh.c w/tools/virsh.c index 276e1cc..4262d60 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -1538,8 +1538,10 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) mac = virXMLPropString(cur, "address"); if (STRCASEEQ(mac, iface)){ + VIR_FREE(mac); goto hit; } + VIR_FREE(mac); } cur = cur->next; } @@ -1573,7 +1575,6 @@ cleanup: if (dom) virDomainFree(dom); - VIR_FREE(mac); return ret; } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2011年12月08日 14:09, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
Detected by valgrind. Leak introduced in commit e9bd9a0:
* tools/virsh.c: fix memory leak on cmdBlkdeviotune.
* how to reproduce? % valgrind -v --leak-check=full virsh blkdeviotune<domain name> <block device>
* actual valgrind result:
==12759== 576 bytes in 1 blocks are definitely lost in loss record 18 of 29 ==12759== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==12759== by 0x42134E: _vshCalloc.clone.2 (virsh.c:422) ==12759== by 0x4217CB: cmdBlkdeviotune (virsh.c:6364) ==12759== by 0x4136A2: vshCommandRun (virsh.c:16363) ==12759== by 0x4253FB: main (virsh.c:17865) ==12759== ==12759== LEAK SUMMARY: ==12759== definitely lost: 576 bytes in 1 blocks ==12759== indirectly lost: 0 bytes in 0 blocks ==12759== possibly lost: 0 bytes in 0 blocks ==12759== still reachable: 126,964 bytes in 1,342 blocks ==12759== suppressed: 0 bytes in 0 blocks
Signed-off-by: Alex Jia<ajia@redhat.com> --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index a51478f..e6e4f8b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6400,8 +6400,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) } }
- virDomainFree(dom); - return true; + ret = true; + goto cleanup; } else { /* Set the block I/O throttle, match by opt since parameters can be 0 */ params = vshCalloc(ctl, nparams, sizeof(*params));
ACK. Osier
On 12/07/2011 11:17 PM, Osier Yang wrote:
On 2011年12月08日 14:09, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
Detected by valgrind. Leak introduced in commit e9bd9a0:
* tools/virsh.c: fix memory leak on cmdBlkdeviotune.
ACK.
Pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
ajia＠redhat.com -
Alex Jia
-
Eric Blake -
Osier Yang