Re: [libvirt] [PATCH] virsh: fix memory leak in cmdVolPath code

2011-07-27 Thread Alex Jia 

On 07/28/2011 11:06 AM, Osier Yang wrote:

于 2011年07月27日 11:48, Osier Yang 写道:

于 2011年07月27日 11:33, Alex Jia 写道:

* tools/virsh.c: avoid memory leak in cmdVolPath.

* how to reproduce?

   % dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
   % virsh pool-refresh default
   % valgrind -v --leak-check=full virsh vol-path --vol
  /var/lib/libvirt/images/foo.img

* actual results:

Detected in valgrind run:

==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 
of 22

==16436==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==16436==by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==16436==by 0x3DF8CD770D: xdr_remote_nonnull_string 
(remote_protocol.c:30)

==16436==by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
(remote_protocol.c:2952)
==16436==by 0x3DF8CDF161: virNetMessageDecodePayload 
(virnetmessage.c:286)

==16436==by 0x3DF8CDE9E5: virNetClientProgramCall
(virnetclientprogram.c:318)
==16436==by 0x3DF8CC28A2: call (remote_driver.c:3929)
==16436==by 0x3DF8CC8412: remoteStorageVolGetPath
(remote_client_bodies.h:5219)
==16436==by 0x3DF8C9BF14: virStorageVolGetPath (libvirt.c:11389)
==16436==by 0x418ECA: cmdVolPath (virsh.c:8754)
==16436==by 0x410CC2: vshCommandRun (virsh.c:12758)
==16436==by 0x41F286: main (virsh.c:14110)
==16436==
==16436== LEAK SUMMARY:
==16436==definitely lost: 32 bytes in 1 blocks


Signed-off-by: Alex Jia
---
  tools/virsh.c |9 -
  1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 841df61..d194a8b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9326,6 +9326,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  {
  virStorageVolPtr vol;
  const char *name = NULL;
+char * StorageVolPath;

  if (!vshConnectionUsability(ctl, ctl->conn))
  return false;
@@ -9334,7 +9335,13 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  return false;
  }

-vshPrint(ctl, "%s\n", virStorageVolGetPath(vol));
+if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
+virStorageVolFree(vol);
+return false;
+}
+
+vshPrint(ctl, "%s\n", StorageVolPath);
+VIR_FREE(StorageVolPath);
  virStorageVolFree(vol);
  return true;
  }


ACK, but it will be better if docs of virStorageVolGetPath in 
src/libvirt.c

can be updated, (to tell the returned path must be freed by caller).

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Pushed with comments of virStorageVolGetPath updated. Thanks

Osier

Osier, thanks for your push and update.

BTW, it seems other comments of function are also missing this 
introduction for whether caller should

free this.

Regards,
Alex

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: fix memory leak in cmdVolPath code

2011-07-27 Thread Osier Yang 

于 2011年07月27日 11:48, Osier Yang 写道:

于 2011年07月27日 11:33, Alex Jia 写道:

* tools/virsh.c: avoid memory leak in cmdVolPath.

* how to reproduce?

   % dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
   % virsh pool-refresh default
   % valgrind -v --leak-check=full virsh vol-path --vol
  /var/lib/libvirt/images/foo.img

* actual results:

Detected in valgrind run:

==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 
of 22

==16436==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==16436==by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==16436==by 0x3DF8CD770D: xdr_remote_nonnull_string 
(remote_protocol.c:30)

==16436==by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
(remote_protocol.c:2952)
==16436==by 0x3DF8CDF161: virNetMessageDecodePayload 
(virnetmessage.c:286)

==16436==by 0x3DF8CDE9E5: virNetClientProgramCall
(virnetclientprogram.c:318)
==16436==by 0x3DF8CC28A2: call (remote_driver.c:3929)
==16436==by 0x3DF8CC8412: remoteStorageVolGetPath
(remote_client_bodies.h:5219)
==16436==by 0x3DF8C9BF14: virStorageVolGetPath (libvirt.c:11389)
==16436==by 0x418ECA: cmdVolPath (virsh.c:8754)
==16436==by 0x410CC2: vshCommandRun (virsh.c:12758)
==16436==by 0x41F286: main (virsh.c:14110)
==16436==
==16436== LEAK SUMMARY:
==16436==definitely lost: 32 bytes in 1 blocks


Signed-off-by: Alex Jia
---
  tools/virsh.c |9 -
  1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 841df61..d194a8b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9326,6 +9326,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  {
  virStorageVolPtr vol;
  const char *name = NULL;
+char * StorageVolPath;

  if (!vshConnectionUsability(ctl, ctl->conn))
  return false;
@@ -9334,7 +9335,13 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  return false;
  }

-vshPrint(ctl, "%s\n", virStorageVolGetPath(vol));
+if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
+virStorageVolFree(vol);
+return false;
+}
+
+vshPrint(ctl, "%s\n", StorageVolPath);
+VIR_FREE(StorageVolPath);
  virStorageVolFree(vol);
  return true;
  }


ACK, but it will be better if docs of virStorageVolGetPath in 
src/libvirt.c

can be updated, (to tell the returned path must be freed by caller).

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Pushed with comments of virStorageVolGetPath updated. Thanks

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: fix memory leak in cmdVolPath code

2011-07-26 Thread Osier Yang 

于 2011年07月27日 11:33, Alex Jia 写道:

* tools/virsh.c: avoid memory leak in cmdVolPath.

* how to reproduce?

   % dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
   % virsh pool-refresh default
   % valgrind -v --leak-check=full virsh vol-path --vol
  /var/lib/libvirt/images/foo.img

* actual results:

Detected in valgrind run:

==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 of 22
==16436==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==16436==by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==16436==by 0x3DF8CD770D: xdr_remote_nonnull_string (remote_protocol.c:30)
==16436==by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
(remote_protocol.c:2952)
==16436==by 0x3DF8CDF161: virNetMessageDecodePayload (virnetmessage.c:286)
==16436==by 0x3DF8CDE9E5: virNetClientProgramCall
(virnetclientprogram.c:318)
==16436==by 0x3DF8CC28A2: call (remote_driver.c:3929)
==16436==by 0x3DF8CC8412: remoteStorageVolGetPath
(remote_client_bodies.h:5219)
==16436==by 0x3DF8C9BF14: virStorageVolGetPath (libvirt.c:11389)
==16436==by 0x418ECA: cmdVolPath (virsh.c:8754)
==16436==by 0x410CC2: vshCommandRun (virsh.c:12758)
==16436==by 0x41F286: main (virsh.c:14110)
==16436==
==16436== LEAK SUMMARY:
==16436==definitely lost: 32 bytes in 1 blocks


Signed-off-by: Alex Jia
---
  tools/virsh.c |9 -
  1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 841df61..d194a8b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9326,6 +9326,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  {
  virStorageVolPtr vol;
  const char *name = NULL;
+char * StorageVolPath;

  if (!vshConnectionUsability(ctl, ctl->conn))
  return false;
@@ -9334,7 +9335,13 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  return false;
  }

-vshPrint(ctl, "%s\n", virStorageVolGetPath(vol));
+if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
+virStorageVolFree(vol);
+return false;
+}
+
+vshPrint(ctl, "%s\n", StorageVolPath);
+VIR_FREE(StorageVolPath);
  virStorageVolFree(vol);
  return true;
  }


ACK, but it will be better if docs of virStorageVolGetPath in src/libvirt.c
can be updated, (to tell the returned path must be freed by caller).

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virsh: fix memory leak in cmdVolPath code

2011-07-26 Thread ajia 
* tools/virsh.c: avoid memory leak in cmdVolPath.

* how to reproduce?

  % dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
  % virsh pool-refresh default
  % valgrind -v --leak-check=full virsh vol-path --vol
 /var/lib/libvirt/images/foo.img

* actual results:

Detected in valgrind run:

==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 of 22
==16436==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==16436==by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==16436==by 0x3DF8CD770D: xdr_remote_nonnull_string (remote_protocol.c:30)
==16436==by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
(remote_protocol.c:2952)
==16436==by 0x3DF8CDF161: virNetMessageDecodePayload (virnetmessage.c:286)
==16436==by 0x3DF8CDE9E5: virNetClientProgramCall
(virnetclientprogram.c:318)
==16436==by 0x3DF8CC28A2: call (remote_driver.c:3929)
==16436==by 0x3DF8CC8412: remoteStorageVolGetPath
(remote_client_bodies.h:5219)
==16436==by 0x3DF8C9BF14: virStorageVolGetPath (libvirt.c:11389)
==16436==by 0x418ECA: cmdVolPath (virsh.c:8754)
==16436==by 0x410CC2: vshCommandRun (virsh.c:12758)
==16436==by 0x41F286: main (virsh.c:14110)
==16436==
==16436== LEAK SUMMARY:
==16436==definitely lost: 32 bytes in 1 blocks


Signed-off-by: Alex Jia 
---
 tools/virsh.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 841df61..d194a8b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9326,6 +9326,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
 {
 virStorageVolPtr vol;
 const char *name = NULL;
+char * StorageVolPath;
 
 if (!vshConnectionUsability(ctl, ctl->conn))
 return false;
@@ -9334,7 +9335,13 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
 return false;
 }
 
-vshPrint(ctl, "%s\n", virStorageVolGetPath(vol));
+if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
+virStorageVolFree(vol);
+return false;
+}
+
+vshPrint(ctl, "%s\n", StorageVolPath);
+VIR_FREE(StorageVolPath);
 virStorageVolFree(vol);
 return true;
 }
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list