From fa8f99bbee93ef05c135ed90d26f0c6437035e91 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Mon, 23 Aug 2021 14:14:55 +0200 Subject: [PATCH] conf: snapshot/checkpoint: Rewrite 'AlignDisk' logic to appease clang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Ján Tomko --- src/conf/checkpoint_conf.c | 14 ++++++++------ src/conf/snapshot_conf.c | 14 ++++++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 4731f21aba..175a95fed7 100644 --- 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 c765e4c815..fc6f0a859d 100644 --- 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;