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

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

 



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@xxxxxxxxxx>
---
  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@xxxxxxxxxx
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]