12:09 a.m.
From: Alex Jia <ajia(a)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(a)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
12:09 a.m.
New subject: [libvirt] [PATCH] virsh: plug memory leak on cmdDomIfGetLink() sucessful path
From: Alex Jia <ajia(a)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(a)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
12:22 a.m.
New subject: [libvirt] [PATCH] virsh: plug memory leak on cmdDomIfGetLink() sucessful path
On 2011年12月08日 14:09, ajia(a)redhat.com wrote:
From: Alex Jia<ajia(a)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(a)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
5:47 p.m.
New subject: [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(a)redhat.com wrote:
> From: Alex Jia<ajia(a)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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:21 p.m.
New subject: [libvirt] [PATCH] virsh: plug memory leak on cmdDomIfGetLink() sucessful path
Oh, It's my fault :(, thanks Eric and Osier.
Regards,
Alex
----- Original Message -----
From: "Eric Blake" <eblake(a)redhat.com>
To: "Osier Yang" <jyang(a)redhat.com>
Cc: ajia(a)redhat.com, libvir-list(a)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(a)redhat.com wrote:
> From: Alex Jia<ajia(a)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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:17 a.m.
On 2011年12月08日 14:09, ajia(a)redhat.com wrote:
From: Alex Jia<ajia(a)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(a)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
5:38 p.m.
On 12/07/2011 11:17 PM, Osier Yang wrote:
On 2011年12月08日 14:09, ajia(a)redhat.com wrote:
> From: Alex Jia<ajia(a)redhat.com>
>
> Detected by valgrind. Leak introduced in commit e9bd9a0:
>
> * tools/virsh.c: fix memory leak on cmdBlkdeviotune.
>
ACK.
Pushed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4762
days inactive
4763
days old
6 comments
4 participants
participants (4)
-
ajia＠redhat.com -
Alex Jia
-
Eric Blake -
Osier Yang