[PULL 05/31] migration: Use explicit error_free() instead of g_autoptr

Peter Xu posted 31 patches 1 month, 2 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Peter Xu <peterx@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Fabiano Rosas <farosas@suse.de>, Mark Kanda <mark.kanda@oracle.com>, Ben Chaney <bchaney@akamai.com>, Eric Blake <eblake@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Laurent Vivier <lvivier@redhat.com>
[PULL 05/31] migration: Use explicit error_free() instead of g_autoptr
Posted by Peter Xu 1 month, 2 weeks ago
There're only two use cases of g_autoptr to free Error objects in migration
code paths.

Due to the nature of how Error should be used (normally ownership will be
passed over to Error APIs, like error_report_err), auto-free functions may
be error prone on its own.  The auto cleanup function was merged without
proper review, as pointed out by Dan and Markus:

https://lore.kernel.org/r/aSWSLMi6ZhTCS_p2@redhat.com

Remove the two use cases so that we can remove the auto cleanup function,
hence suggest to not use auto frees for Errors.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-2-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/multifd-device-state.c | 3 ++-
 migration/savevm.c               | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/multifd-device-state.c b/migration/multifd-device-state.c
index fce64f00b0..db3239fef5 100644
--- a/migration/multifd-device-state.c
+++ b/migration/multifd-device-state.c
@@ -140,7 +140,7 @@ static void multifd_device_state_save_thread_data_free(void *opaque)
 static int multifd_device_state_save_thread(void *opaque)
 {
     SaveCompletePrecopyThreadData *data = opaque;
-    g_autoptr(Error) local_err = NULL;
+    Error *local_err = NULL;
 
     if (!data->hdlr(data, &local_err)) {
         MigrationState *s = migrate_get_current();
@@ -159,6 +159,7 @@ static int multifd_device_state_save_thread(void *opaque)
          * return we end setting is purely arbitrary.
          */
         migrate_set_error(s, local_err);
+        error_free(local_err);
     }
 
     return 0;
diff --git a/migration/savevm.c b/migration/savevm.c
index 62cc2ce25c..638e9b364f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2823,7 +2823,7 @@ static int qemu_loadvm_load_thread(void *thread_opaque)
 {
     struct LoadThreadData *data = thread_opaque;
     MigrationIncomingState *mis = migration_incoming_get_current();
-    g_autoptr(Error) local_err = NULL;
+    Error *local_err = NULL;
 
     if (!data->function(data->opaque, &mis->load_threads_abort, &local_err)) {
         MigrationState *s = migrate_get_current();
@@ -2841,6 +2841,7 @@ static int qemu_loadvm_load_thread(void *thread_opaque)
          * return we end setting is purely arbitrary.
          */
         migrate_set_error(s, local_err);
+        error_free(local_err);
     }
 
     return 0;
-- 
2.50.1