Re: [libvirt] [PATCH 03/14] storage: Resolve resource leak using 'vol' buffer

2013-01-10 Thread Eric Blake 
On 01/09/2013 07:54 AM, John Ferlan wrote:
> ---
>  src/storage/storage_backend_rbd.c | 13 ++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 

>  vol->name = strdup(name);
> -if (vol->name == NULL)
> +if (vol->name == NULL) {
> +VIR_FREE(vol);
>  goto out_of_memory;
> +}
>  
> -if (STREQ(vol->name, ""))
> +if (STREQ(vol->name, "")) {
> +VIR_FREE(vol->name);
> +VIR_FREE(vol);
>  break;
> +}

It feels a bit awkward having to free more and more for each break.  I
rearranged the code slightly to hoist the validation prior to the
malloc.  ACK and pushed with this squashed in:

diff --git i/src/storage/storage_backend_rbd.c
w/src/storage/storage_backend_rbd.c
index f916de1..8a0e517 100644
--- i/src/storage/storage_backend_rbd.c
+++ w/src/storage/storage_backend_rbd.c
@@ -1,6 +1,7 @@
 /*
  * storage_backend_rbd.c: storage backend for RBD (RADOS Block Device)
handling
  *
+ * Copyright (C) 2013 Red Hat, Inc.
  * Copyright (C) 2012 Wido den Hollander
  *
  * This library is free software; you can redistribute it and/or
@@ -319,12 +320,16 @@ static int
virStorageBackendRBDRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
 }

 for (i = 0, name = names; name < names + max_size; i++) {
+virStorageVolDefPtr vol;
+
 if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count + 1)
< 0) {
 virStoragePoolObjClearVols(pool);
 goto out_of_memory;
 }

-virStorageVolDefPtr vol;
+if (STREQ(name, ""))
+break;
+
 if (VIR_ALLOC(vol) < 0)
 goto out_of_memory;

@@ -334,12 +339,6 @@ static int
virStorageBackendRBDRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
 goto out_of_memory;
 }

-if (STREQ(vol->name, "")) {
-VIR_FREE(vol->name);
-VIR_FREE(vol);
-break;
-}
-
 name += strlen(name) + 1;

 if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr) < 0) {


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 03/14] storage: Resolve resource leak using 'vol' buffer

2013-01-09 Thread John Ferlan 
---
 src/storage/storage_backend_rbd.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index f5c6b0f..f916de1 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -329,16 +329,23 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr 
conn ATTRIBUTE_UNUSED,
 goto out_of_memory;
 
 vol->name = strdup(name);
-if (vol->name == NULL)
+if (vol->name == NULL) {
+VIR_FREE(vol);
 goto out_of_memory;
+}
 
-if (STREQ(vol->name, ""))
+if (STREQ(vol->name, "")) {
+VIR_FREE(vol->name);
+VIR_FREE(vol);
 break;
+}
 
 name += strlen(name) + 1;
 
-if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr) < 0)
+if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr) < 0) {
+virStorageVolDefFree(vol);
 goto cleanup;
+}
 
 pool->volumes.objs[pool->volumes.count++] = vol;
 }
-- 
1.7.11.7

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