[PATCH 3/4] conf: snapshot: Refactor virDomainSnapshotDefAssignExternalNames

[Peter Krempa <pkrempa@xxxxxxxxxx>]

Get rid of one indentation level by negating condition and remove ugly
pointer arithmetic at the cost of one extra allocation.
---
 src/conf/snapshot_conf.c | 94 +++++++++++++++++++++++-------------------------
 1 file changed, 44 insertions(+), 50 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index da6fec5..f8a1aed 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -440,66 +440,60 @@ virDomainSnapshotDefParseString(const char *xmlStr,
 static int
 virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDefPtr def)
 {
+    const char *origpath;
+    char *tmppath;
+    char *tmp;
+    struct stat sb;
     size_t i;
-    int ret = -1;

     for (i = 0; i < def->ndisks; i++) {
         virDomainSnapshotDiskDefPtr disk = &def->disks[i];

-        if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL &&
-            !disk->src->path) {
-            const char *original = virDomainDiskGetSource(def->dom->disks[i]);
-            const char *tmp;
-            struct stat sb;
-
-            if (disk->src->type != VIR_STORAGE_TYPE_FILE) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("cannot generate external snapshot name "
-                                 "for disk '%s' on a '%s' device"),
-                               disk->name,
-                               virStorageTypeToString(disk->src->type));
-                goto cleanup;
-            }
+        if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL ||
+            disk->src->path)
+            continue;

-            if (!original) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("cannot generate external snapshot name "
-                                 "for disk '%s' without source"),
-                               disk->name);
-                goto cleanup;
-            }
-            if (stat(original, &sb) < 0 || !S_ISREG(sb.st_mode)) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("source for disk '%s' is not a regular "
-                                 "file; refusing to generate external "
-                                 "snapshot name"),
-                               disk->name);
-                goto cleanup;
-            }
+        if (disk->src->type != VIR_STORAGE_TYPE_FILE) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("cannot generate external snapshot name "
+                             "for disk '%s' on a '%s' device"),
+                           disk->name, virStorageTypeToString(disk->src->type));
+            return -1;
+        }

-            tmp = strrchr(original, '.');
-            if (!tmp || strchr(tmp, '/')) {
-                if (virAsprintf(&disk->src->path, "%s.%s", original,
-                                def->name) < 0)
-                    goto cleanup;
-            } else {
-                if ((tmp - original) > INT_MAX) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("integer overflow"));
-                    goto cleanup;
-                }
-                if (virAsprintf(&disk->src->path, "%.*s.%s",
-                                (int) (tmp - original), original,
-                                def->name) < 0)
-                    goto cleanup;
-            }
+        if (!(origpath = virDomainDiskGetSource(def->dom->disks[i]))) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("cannot generate external snapshot name "
+                             "for disk '%s' without source"),
+                           disk->name);
+            return -1;
         }
-    }

-    ret = 0;
+        if (stat(origpath, &sb) < 0 || !S_ISREG(sb.st_mode)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("source for disk '%s' is not a regular "
+                             "file; refusing to generate external "
+                             "snapshot name"),
+                           disk->name);
+            return -1;
+        }

- cleanup:
-    return ret;
+        if (VIR_STRDUP(tmppath, origpath) < 0)
+            return -1;
+
+        /* drop suffix of the file name */
+        if ((tmp = strrchr(tmppath, '.')) && !strchr(tmp, '/'))
+            *tmp = '\0';
+
+        if (virAsprintf(&disk->src->path, "%s.%s", tmppath, def->name) < 0) {
+            VIR_FREE(tmppath);
+            return -1;
+        }
+
+        VIR_FREE(tmppath);
+    }
+
+    return 0;
 }


-- 
2.6.2

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