conf: snapshot/checkpoint: Rewrite 'AlignDisk' logic to appease clang

New clang has a false-positive about value of 'olddisks' being unused
after being set. This is clearly wrong because we want to use
'g_autofree' to clear it later.

While I'm against modifying good code for the sake of bad static
analysis in this case it's not obvious that we depend on the lifetime of
'olddisks' being needed until the end of the function as we store
pointers into it into the hash table and later copy them out.

Rewrite the code by assigning to 'olddisks' earlier and then using
'olddisks' in the loop, so it's clear where the lifetime of the objects
ends, and this should also silence the warning.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
index 4731f21aba17f77e1e7b9850d8ae5b7cd1815864..175a95fed7163866da705634355e180e8a72374e 100644 (file)
--- a/src/conf/checkpoint_conf.c
+++ b/src/conf/checkpoint_conf.c
@@ -265,6 +265,7 @@ virDomainCheckpointAlignDisks(virDomainCheckpointDef *chkdef)
     virDomainDef *domdef = chkdef->parent.dom;
     g_autoptr(GHashTable) map = virHashNew(NULL);
     g_autofree virDomainCheckpointDiskDef *olddisks = NULL;
+    size_t oldndisks;
     size_t i;
     int checkpoint_default = VIR_DOMAIN_CHECKPOINT_TYPE_NONE;
 
@@ -292,9 +293,14 @@ virDomainCheckpointAlignDisks(virDomainCheckpointDef *chkdef)
     if (!chkdef->ndisks)
         checkpoint_default = VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP;
 
+    olddisks = g_steal_pointer(&chkdef->disks);
+    oldndisks = chkdef->ndisks;
+    chkdef->disks = g_new0(virDomainCheckpointDiskDef, domdef->ndisks);
+    chkdef->ndisks = domdef->ndisks;
+
     /* Double check requested disks.  */
-    for (i = 0; i < chkdef->ndisks; i++) {
-        virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i];
+    for (i = 0; i < oldndisks; i++) {
+        virDomainCheckpointDiskDef *chkdisk = &olddisks[i];
         virDomainDiskDef *domdisk = virDomainDiskByName(domdef, chkdisk->name, false);
 
         if (!domdisk) {
@@ -328,10 +334,6 @@ virDomainCheckpointAlignDisks(virDomainCheckpointDef *chkdef)
         }
     }
 
-    olddisks = g_steal_pointer(&chkdef->disks);
-    chkdef->disks = g_new0(virDomainCheckpointDiskDef, domdef->ndisks);
-    chkdef->ndisks = domdef->ndisks;
-
     for (i = 0; i < domdef->ndisks; i++) {
         virDomainDiskDef *domdisk = domdef->disks[i];
         virDomainCheckpointDiskDef *chkdisk = chkdef->disks + i;

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index c765e4c815a292f257173c6970114f95a0d6fa4e..fc6f0a859de98be9384e31442be8bd3205e9de3c 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -636,6 +636,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
     virDomainDef *domdef = snapdef->parent.dom;
     g_autoptr(GHashTable) map = virHashNew(NULL);
     g_autofree virDomainSnapshotDiskDef *olddisks = NULL;
+    size_t oldndisks;
     size_t i;
 
     if (!domdef) {
@@ -654,9 +655,14 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
     if (!domdef->ndisks)
         return 0;
 
+    olddisks = g_steal_pointer(&snapdef->disks);
+    oldndisks = snapdef->ndisks;
+    snapdef->disks = g_new0(virDomainSnapshotDiskDef, domdef->ndisks);
+    snapdef->ndisks = domdef->ndisks;
+
     /* Double check requested disks.  */
-    for (i = 0; i < snapdef->ndisks; i++) {
-        virDomainSnapshotDiskDef *snapdisk = &snapdef->disks[i];
+    for (i = 0; i < oldndisks; i++) {
+        virDomainSnapshotDiskDef *snapdisk = &olddisks[i];
         virDomainDiskDef *domdisk = virDomainDiskByName(domdef, snapdisk->name, false);
 
         if (!domdisk) {
@@ -708,10 +714,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
         }
     }
 
-    olddisks = g_steal_pointer(&snapdef->disks);
-    snapdef->disks = g_new0(virDomainSnapshotDiskDef, domdef->ndisks);
-    snapdef->ndisks = domdef->ndisks;
-
     for (i = 0; i < domdef->ndisks; i++) {
         virDomainDiskDef *domdisk = domdef->disks[i];
         virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i;