Some compilers aren't happy when an automatically freed variable is used
just to free something (thus it's only assigned in the code):

When compiling qemuSnapshotDelete after recent commits they complain:

../src/qemu/qemu_snapshot.c:3153:61: error: variable 'delData' set but not used [-Werror,-Wunused-but-set-variable]
                g_autoslist(qemuSnapshotDeleteExternalData) delData = NULL;
                                                            ^

To work around the issue we can restructure the code which also has the
following semantic implications:
 - since qemuSnapshotDeleteExternalPrepare does validation we error out
   sooner than attempting to start the VM

 - we read the temporary variable at least in one code path

Fixes: 4a4d89a9252
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_snapshot.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 32f7011cbe..b8416808b3 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3146,12 +3146,13 @@ qemuSnapshotDelete(virDomainObj *vm,
         if (virDomainSnapshotIsExternal(snap) &&
             !(flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
                        VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY))) {
+            g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL;

-            externalData = qemuSnapshotDeleteExternalPrepare(vm, snap);
+            /* this also serves as validation whether the snapshot can be deleted */
+            if (!(tmpData = qemuSnapshotDeleteExternalPrepare(vm, snap)))
+                goto endjob;

             if (!virDomainObjIsActive(vm)) {
-                g_autoslist(qemuSnapshotDeleteExternalData) delData = NULL;
-
                 if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT,
                                      NULL, -1, NULL, NULL,
                                      VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
@@ -3163,20 +3164,19 @@ qemuSnapshotDelete(virDomainObj *vm,

                 /* Call the prepare again as some data require that the VM is
                  * running to get everything we need. */
-                delData = g_steal_pointer(&externalData);
-                externalData = qemuSnapshotDeleteExternalPrepare(vm, snap);
+                if (!(externalData = qemuSnapshotDeleteExternalPrepare(vm, snap)))
+                    goto endjob;
             } else {
                 qemuDomainJobPrivate *jobPriv = vm->job->privateData;

+                externalData = g_steal_pointer(&tmpData);
+
                 /* If the VM is running we need to indicate that the async snapshot
                  * job is snapshot delete job. */
                 jobPriv->snapshotDelete = true;

                 qemuDomainSaveStatus(vm);
             }
-
-            if (!externalData)
-                goto endjob;
         }
     }

-- 
2.38.1

On Mon, Jan 09, 2023 at 02:09:24PM +0100, Peter Krempa wrote:
> Some compilers aren't happy when an automatically freed variable is used
> just to free something (thus it's only assigned in the code):
> 
> When compiling qemuSnapshotDelete after recent commits they complain:
> 
> ../src/qemu/qemu_snapshot.c:3153:61: error: variable 'delData' set but not used [-Werror,-Wunused-but-set-variable]
>                 g_autoslist(qemuSnapshotDeleteExternalData) delData = NULL;
>                                                             ^
> 
> To work around the issue we can restructure the code which also has the
> following semantic implications:
>  - since qemuSnapshotDeleteExternalPrepare does validation we error out
>    sooner than attempting to start the VM
> 
>  - we read the temporary variable at least in one code path
> 
> Fixes: 4a4d89a9252
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>