[libvirt] [PATCH 07/12] storage_conf: Improve the memory deallocation of pool def parsing

Osier Yang jyang at redhat.com
Wed May 22 12:05:17 UTC 2013


Changes:
    * Free all the strings at "cleanup", instead of freeing them
      in the middle
    * Remove xmlFree
    * s/tmppath/target_path/, to make it more sensible
    * Add new goto label "error"
---
 src/conf/storage_conf.c | 54 ++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index efe02e8..6f0ed74 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -820,7 +820,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
     xmlNodePtr source_node;
     char *type = NULL;
     char *uuid = NULL;
-    char *tmppath;
+    char *target_path = NULL;
 
     if (VIR_ALLOC(ret) < 0) {
         virReportOOMError();
@@ -831,21 +831,18 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
     if ((ret->type = virStoragePoolTypeFromString(type)) < 0) {
         virReportError(VIR_ERR_XML_ERROR,
                        _("unknown storage pool type %s"), type);
-        goto cleanup;
+        goto error;
     }
 
-    xmlFree(type);
-    type = NULL;
-
     if ((options = virStoragePoolOptionsForPoolType(ret->type)) == NULL) {
-        goto cleanup;
+        goto error;
     }
 
     source_node = virXPathNode("./source", ctxt);
     if (source_node) {
         if (virStoragePoolDefParseSource(ctxt, &ret->source, ret->type,
                                          source_node) < 0)
-            goto cleanup;
+            goto error;
     }
 
     ret->name = virXPathString("string(./name)", ctxt);
@@ -855,7 +852,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
     if (ret->name == NULL) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("missing pool source name element"));
-        goto cleanup;
+        goto error;
     }
 
     uuid = virXPathString("string(./uuid)", ctxt);
@@ -863,22 +860,21 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
         if (virUUIDGenerate(ret->uuid) < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("unable to generate uuid"));
-            goto cleanup;
+            goto error;
         }
     } else {
         if (virUUIDParse(uuid, ret->uuid) < 0) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("malformed uuid element"));
-            goto cleanup;
+            goto error;
         }
-        VIR_FREE(uuid);
     }
 
     if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) {
         if (!ret->source.nhost) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("missing storage pool source host name"));
-            goto cleanup;
+            goto error;
         }
     }
 
@@ -886,7 +882,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
         if (!ret->source.dir) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("missing storage pool source path"));
-            goto cleanup;
+            goto error;
         }
     }
     if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) {
@@ -895,7 +891,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
             ret->source.name = strdup(ret->name);
             if (ret->source.name == NULL) {
                 virReportOOMError();
-                goto cleanup;
+                goto error;
             }
         }
     }
@@ -904,7 +900,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
         if (!ret->source.adapter.type) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("missing storage pool source adapter"));
-            goto cleanup;
+            goto error;
         }
 
         if (ret->source.adapter.type ==
@@ -914,7 +910,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
                 virReportError(VIR_ERR_XML_ERROR, "%s",
                                _("'wwnn' and 'wwpn' must be specified for adapter "
                                  "type 'fchost'"));
-                goto cleanup;
+                goto error;
             }
 
             if (!virValidateWWN(ret->source.adapter.data.fchost.wwnn) ||
@@ -925,7 +921,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
             if (!ret->source.adapter.data.name) {
                 virReportError(VIR_ERR_XML_ERROR, "%s",
                                _("missing storage pool source adapter name"));
-                goto cleanup;
+                goto error;
             }
         }
     }
@@ -935,36 +931,38 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
         if (!ret->source.ndevice) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("missing storage pool source device name"));
-            goto cleanup;
+            goto error;
         }
     }
 
     /* When we are working with a virtual disk we can skip the target
      * path and permissions */
     if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) {
-        if ((tmppath = virXPathString("string(./target/path)", ctxt)) == NULL) {
+        if (!(target_path = virXPathString("string(./target/path)", ctxt))) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("missing storage pool target path"));
-            goto cleanup;
+            goto error;
         }
-        ret->target.path = virFileSanitizePath(tmppath);
-        VIR_FREE(tmppath);
+        ret->target.path = virFileSanitizePath(target_path);
         if (!ret->target.path)
-            goto cleanup;
+            goto error;
 
         if (virStorageDefParsePerms(ctxt, &ret->target.perms,
                                     "./target/permissions",
                                     DEFAULT_POOL_PERM_MODE) < 0)
-            goto cleanup;
+            goto error;
     }
 
-    return ret;
-
 cleanup:
     VIR_FREE(uuid);
-    xmlFree(type);
+    VIR_FREE(type);
+    VIR_FREE(target_path);
+    return ret;
+
+error:
     virStoragePoolDefFree(ret);
-    return NULL;
+    ret = NULL;
+    goto cleanup;
 }
 
 virStoragePoolDefPtr
-- 
1.8.1.4




More information about the libvir-list mailing list