[PATCH 4/5] storage_conf: Left fixes or improvements for storage_conf.c

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

 



virStorageVolDefParseXML:
  * Create "virStorageVolDefPtr def", and use ret to track the
    return value; frees the strings at "cleanup" label instead
    of freeing them in the middle.

virStorageVolDefFormat:
  * Use macro NULLSTR
---
 src/conf/storage_conf.c | 93 ++++++++++++++++++++++++-------------------------
 1 file changed, 46 insertions(+), 47 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index dd55d2c..431a8eb 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -972,8 +972,8 @@ virStoragePoolDefParseNode(xmlDocPtr xml,
     virStoragePoolDefPtr def = NULL;
 
     if (STRNEQ((const char *)root->name, "pool")) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       "%s", _("unknown root element for storage pool"));
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("unknown root element for storage pool"));
         goto cleanup;
     }
 
@@ -1149,8 +1149,8 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
 
     type = virStoragePoolTypeToString(def->type);
     if (!type) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("unexpected pool type"));
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("unexpected pool type"));
         goto cleanup;
     }
     virBufferAsprintf(&buf, "<pool type='%s'>\n", type);
@@ -1214,8 +1214,8 @@ virStorageSize(const char *unit,
                unsigned long long *ret)
 {
     if (virStrToLong_ull(val, NULL, 10, ret) < 0) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       "%s", _("malformed capacity element"));
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("malformed capacity element"));
         return -1;
     }
     /* off_t is signed, so you cannot create a file larger than 2**63
@@ -1230,64 +1230,62 @@ static virStorageVolDefPtr
 virStorageVolDefParseXML(virStoragePoolDefPtr pool,
                          xmlXPathContextPtr ctxt)
 {
-    virStorageVolDefPtr ret;
+    virStorageVolDefPtr def;
     virStorageVolOptionsPtr options;
     char *allocation = NULL;
     char *capacity = NULL;
     char *unit = NULL;
     xmlNodePtr node;
+    virStorageVolDefPtr ret = NULL;
 
     options = virStorageVolOptionsForPoolType(pool->type);
     if (options == NULL)
         return NULL;
 
-    if (VIR_ALLOC(ret) < 0) {
+    if (VIR_ALLOC(def) < 0) {
         virReportOOMError();
         return NULL;
     }
 
-    ret->name = virXPathString("string(./name)", ctxt);
-    if (ret->name == NULL) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       "%s", _("missing volume name element"));
+    def->name = virXPathString("string(./name)", ctxt);
+    if (def->name == NULL) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing volume name element"));
         goto cleanup;
     }
 
     /* Auto-generated so deliberately ignore */
-    /* ret->key = virXPathString("string(./key)", ctxt); */
+    /* def->key = virXPathString("string(./key)", ctxt); */
 
     capacity = virXPathString("string(./capacity)", ctxt);
     unit = virXPathString("string(./capacity/@unit)", ctxt);
     if (capacity == NULL) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       "%s", _("missing capacity element"));
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing capacity element"));
         goto cleanup;
     }
-    if (virStorageSize(unit, capacity, &ret->capacity) < 0)
+    if (virStorageSize(unit, capacity, &def->capacity) < 0)
         goto cleanup;
-    VIR_FREE(capacity);
     VIR_FREE(unit);
 
     allocation = virXPathString("string(./allocation)", ctxt);
     if (allocation) {
         unit = virXPathString("string(./allocation/@unit)", ctxt);
-        if (virStorageSize(unit, allocation, &ret->allocation) < 0)
+        if (virStorageSize(unit, allocation, &def->allocation) < 0)
             goto cleanup;
-        VIR_FREE(allocation);
-        VIR_FREE(unit);
     } else {
-        ret->allocation = ret->capacity;
+        def->allocation = def->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_XML_ERROR,
                            _("unknown volume format type %s"), format);
             VIR_FREE(format);
@@ -1296,28 +1294,28 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
         VIR_FREE(format);
     }
 
-    if (virStorageDefParsePerms(ctxt, &ret->target.perms,
+    if (virStorageDefParsePerms(ctxt, &def->target.perms,
                                 "./target/permissions",
                                 DEFAULT_VOL_PERM_MODE) < 0)
         goto cleanup;
 
     node = virXPathNode("./target/encryption", ctxt);
     if (node != NULL) {
-        ret->target.encryption = virStorageEncryptionParseNode(ctxt->doc,
+        def->target.encryption = virStorageEncryptionParseNode(ctxt->doc,
                                                                node);
-        if (ret->target.encryption == NULL)
+        if (def->target.encryption == NULL)
             goto cleanup;
     }
 
-    ret->backingStore.path = virXPathString("string(./backingStore/path)", ctxt);
+    def->backingStore.path = virXPathString("string(./backingStore/path)", ctxt);
     if (options->formatFromString) {
         char *format = virXPathString("string(./backingStore/format/@type)", ctxt);
         if (format == NULL)
-            ret->backingStore.format = options->defaultFormat;
+            def->backingStore.format = options->defaultFormat;
         else
-            ret->backingStore.format = (options->formatFromString)(format);
+            def->backingStore.format = (options->formatFromString)(format);
 
-        if (ret->backingStore.format < 0) {
+        if (def->backingStore.format < 0) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("unknown volume format type %s"), format);
             VIR_FREE(format);
@@ -1326,19 +1324,19 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
         VIR_FREE(format);
     }
 
-    if (virStorageDefParsePerms(ctxt, &ret->backingStore.perms,
+    if (virStorageDefParsePerms(ctxt, &def->backingStore.perms,
                                 "./backingStore/permissions",
                                 DEFAULT_VOL_PERM_MODE) < 0)
         goto cleanup;
 
-    return ret;
-
+    ret = def;
 cleanup:
     VIR_FREE(allocation);
     VIR_FREE(capacity);
     VIR_FREE(unit);
-    virStorageVolDefFree(ret);
-    return NULL;
+    if (!ret)
+        virStorageVolDefFree(ret);
+    return ret;
 }
 
 virStorageVolDefPtr
@@ -1350,8 +1348,8 @@ virStorageVolDefParseNode(virStoragePoolDefPtr pool,
     virStorageVolDefPtr def = NULL;
 
     if (STRNEQ((const char *)root->name, "volume")) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       "%s", _("unknown root element for storage vol"));
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("unknown root element for storage vol"));
         goto cleanup;
     }
 
@@ -1481,7 +1479,7 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
 
     virBufferAddLit(&buf, "<volume>\n");
     virBufferAsprintf(&buf,"  <name>%s</name>\n", def->name);
-    virBufferAsprintf(&buf,"  <key>%s</key>\n", def->key ? def->key : "(null)");
+    virBufferAsprintf(&buf,"  <key>%s</key>\n", NULLSTR(def->key));
     virBufferAddLit(&buf, "  <source>\n");
 
     if (def->source.nextent) {
@@ -1658,8 +1656,8 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
     }
 
     if (virMutexInit(&pool->lock) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("cannot initialize mutex"));
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("cannot initialize mutex"));
         VIR_FREE(pool);
         return NULL;
     }
@@ -1694,7 +1692,8 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
 
     if (!virFileMatchesNameSuffix(file, def->name, ".xml")) {
         virReportError(VIR_ERR_XML_ERROR,
-                       _("Storage pool config filename '%s' does not match pool name '%s'"),
+                       _("Storage pool config filename '%s' "
+                         "does not match pool name '%s'"),
                        path, def->name);
         virStoragePoolDefFree(def);
         return NULL;
@@ -1807,8 +1806,8 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
     }
 
     if (!(xml = virStoragePoolDefFormat(def))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("failed to generate XML"));
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("failed to generate XML"));
         return -1;
     }
 
@@ -1870,8 +1869,8 @@ virStoragePoolSourceListFormat(virStoragePoolSourceListPtr def)
 
     type = virStoragePoolTypeToString(def->type);
     if (!type) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("unexpected pool type"));
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("unexpected pool type"));
         goto cleanup;
     }
 
-- 
1.8.1.4

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