[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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<ajia redhat com>
---
  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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]