[PATCH v2 06/32] conf: Rework virStorageVolDefParseXML

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

 



Rather than having an error path, let's rework the code to allocate
and fill into an @def variable and then steal that into @ret when we
are successful leaving just a cleanup: path.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
---
 src/conf/storage_conf.c | 113 ++++++++++++++++++++--------------------
 1 file changed, 56 insertions(+), 57 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 9563d2bc6b..0ebdbafa0c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1168,7 +1168,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
                          xmlXPathContextPtr ctxt,
                          unsigned int flags)
 {
-    virStorageVolDefPtr ret;
+    virStorageVolDefPtr def;
+    virStorageVolDefPtr ret = NULL;
     virStorageVolOptionsPtr options;
     char *type = NULL;
     char *allocation = NULL;
@@ -1187,132 +1188,132 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
     if (options == NULL)
         return NULL;
 
-    if (VIR_ALLOC(ret) < 0)
+    if (VIR_ALLOC(def) < 0)
         return NULL;
 
-    ret->target.type = VIR_STORAGE_TYPE_FILE;
+    def->target.type = VIR_STORAGE_TYPE_FILE;
 
-    ret->name = virXPathString("string(./name)", ctxt);
-    if (ret->name == NULL) {
+    def->name = virXPathString("string(./name)", ctxt);
+    if (def->name == NULL) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("missing volume name element"));
-        goto error;
+        goto cleanup;
     }
 
     /* Normally generated by pool refresh, but useful for unit tests */
-    ret->key = virXPathString("string(./key)", ctxt);
+    def->key = virXPathString("string(./key)", ctxt);
 
     /* Technically overridden by pool refresh, but useful for unit tests */
     type = virXPathString("string(./@type)", ctxt);
     if (type) {
-        if ((ret->type = virStorageVolTypeFromString(type)) < 0) {
+        if ((def->type = virStorageVolTypeFromString(type)) < 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("unknown volume type '%s'"), type);
-            goto error;
+            goto cleanup;
         }
     }
 
     if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) {
-        if (VIR_ALLOC(ret->target.backingStore) < 0)
-            goto error;
+        if (VIR_ALLOC(def->target.backingStore) < 0)
+            goto cleanup;
 
-        ret->target.backingStore->type = VIR_STORAGE_TYPE_FILE;
+        def->target.backingStore->type = VIR_STORAGE_TYPE_FILE;
 
-        ret->target.backingStore->path = backingStore;
+        def->target.backingStore->path = backingStore;
         backingStore = NULL;
 
         if (options->formatFromString) {
             char *format = virXPathString("string(./backingStore/format/@type)", ctxt);
             if (format == NULL)
-                ret->target.backingStore->format = options->defaultFormat;
+                def->target.backingStore->format = options->defaultFormat;
             else
-                ret->target.backingStore->format = (options->formatFromString)(format);
+                def->target.backingStore->format = (options->formatFromString)(format);
 
-            if (ret->target.backingStore->format < 0) {
+            if (def->target.backingStore->format < 0) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("unknown volume format type %s"), format);
                 VIR_FREE(format);
-                goto error;
+                goto cleanup;
             }
             VIR_FREE(format);
         }
 
-        if (VIR_ALLOC(ret->target.backingStore->perms) < 0)
-            goto error;
-        if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms,
+        if (VIR_ALLOC(def->target.backingStore->perms) < 0)
+            goto cleanup;
+        if (virStorageDefParsePerms(ctxt, def->target.backingStore->perms,
                                     "./backingStore/permissions") < 0)
-            goto error;
+            goto cleanup;
     }
 
     capacity = virXPathString("string(./capacity)", ctxt);
     unit = virXPathString("string(./capacity/@unit)", ctxt);
     if (capacity) {
-        if (virStorageSize(unit, capacity, &ret->target.capacity) < 0)
-            goto error;
+        if (virStorageSize(unit, capacity, &def->target.capacity) < 0)
+            goto cleanup;
     } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) &&
                !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
-                 virStorageSourceHasBacking(&ret->target))) {
+                 virStorageSourceHasBacking(&def->target))) {
         virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element"));
-        goto error;
+        goto cleanup;
     }
     VIR_FREE(unit);
 
     allocation = virXPathString("string(./allocation)", ctxt);
     if (allocation) {
         unit = virXPathString("string(./allocation/@unit)", ctxt);
-        if (virStorageSize(unit, allocation, &ret->target.allocation) < 0)
-            goto error;
-        ret->target.has_allocation = true;
+        if (virStorageSize(unit, allocation, &def->target.allocation) < 0)
+            goto cleanup;
+        def->target.has_allocation = true;
     } else {
-        ret->target.allocation = ret->target.capacity;
+        def->target.allocation = def->target.capacity;
     }
 
-    ret->target.path = virXPathString("string(./target/path)", ctxt);
+    def->target.path = virXPathString("string(./target/path)", ctxt);
     if (options->formatFromString) {
         char *format = virXPathString("string(./target/format/@type)", ctxt);
         if (format == NULL)
-            ret->target.format = options->defaultFormat;
+            def->target.format = options->defaultFormat;
         else
-            ret->target.format = (options->formatFromString)(format);
+            def->target.format = (options->formatFromString)(format);
 
-        if (ret->target.format < 0) {
+        if (def->target.format < 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("unknown volume format type %s"), format);
             VIR_FREE(format);
-            goto error;
+            goto cleanup;
         }
         VIR_FREE(format);
     }
 
-    if (VIR_ALLOC(ret->target.perms) < 0)
-        goto error;
-    if (virStorageDefParsePerms(ctxt, ret->target.perms,
+    if (VIR_ALLOC(def->target.perms) < 0)
+        goto cleanup;
+    if (virStorageDefParsePerms(ctxt, def->target.perms,
                                 "./target/permissions") < 0)
-        goto error;
+        goto cleanup;
 
     node = virXPathNode("./target/encryption", ctxt);
     if (node != NULL) {
-        ret->target.encryption = virStorageEncryptionParseNode(node, ctxt);
-        if (ret->target.encryption == NULL)
-            goto error;
+        def->target.encryption = virStorageEncryptionParseNode(node, ctxt);
+        if (def->target.encryption == NULL)
+            goto cleanup;
     }
 
-    ret->target.compat = virXPathString("string(./target/compat)", ctxt);
-    if (virStorageFileCheckCompat(ret->target.compat) < 0)
-        goto error;
+    def->target.compat = virXPathString("string(./target/compat)", ctxt);
+    if (virStorageFileCheckCompat(def->target.compat) < 0)
+        goto cleanup;
 
     if (virXPathNode("./target/nocow", ctxt))
-        ret->target.nocow = true;
+        def->target.nocow = true;
 
     if (virXPathNode("./target/features", ctxt)) {
         if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0)
-            goto error;
+            goto cleanup;
 
-        if (!ret->target.compat && VIR_STRDUP(ret->target.compat, "1.1") < 0)
-            goto error;
+        if (!def->target.compat && VIR_STRDUP(def->target.compat, "1.1") < 0)
+            goto cleanup;
 
-        if (!(ret->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST)))
-            goto error;
+        if (!(def->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST)))
+            goto cleanup;
 
         for (i = 0; i < n; i++) {
             int f = virStorageFileFeatureTypeFromString((const char*)nodes[i]->name);
@@ -1320,14 +1321,17 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
             if (f < 0) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported feature %s"),
                                (const char*)nodes[i]->name);
-                goto error;
+                goto cleanup;
             }
-            ignore_value(virBitmapSetBit(ret->target.features, f));
+            ignore_value(virBitmapSetBit(def->target.features, f));
         }
         VIR_FREE(nodes);
     }
 
+    VIR_STEAL_PTR(ret, def);
+
  cleanup:
+    virStorageVolDefFree(def);
     VIR_FREE(nodes);
     VIR_FREE(allocation);
     VIR_FREE(capacity);
@@ -1335,11 +1339,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
     VIR_FREE(type);
     VIR_FREE(backingStore);
     return ret;
-
- error:
-    virStorageVolDefFree(ret);
-    ret = NULL;
-    goto cleanup;
 }
 
 
-- 
2.20.1


[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]

  Powered by Linux