On Fri, 9 Jan 2026 at 18:19, Fabiano Rosas <[email protected]> wrote:
> When setting a callback on a Glib source and giving it a data pointer,
> it's natural to also provide the destructor for the data in question.
>
> Since migrate_hup_add() already needs to clone the MigrationAddress
> when setting the qmp_migrate_finish_cb callback, also pass the
> qapi_free_MigrationAddress as the GDestroyNotify callback.
>
> With this the address doesn't need to be freed at the callback body,
> making the management of that memory slight simpler.

* slight -> slightly  OR  just skip it. ->  ... memory simpler.

> (also fix the indentation of migrate_hup_add)

* This note could be purged.

> Cc: Mark Kanda <[email protected]>
> Cc: Ben Chaney <[email protected]>
> Reviewed-by: Peter Xu <[email protected]>
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
>  migration/migration.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 52c1bb5da2..5167233f76 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2007,9 +2007,11 @@ static void qmp_migrate_finish(MigrationAddress *addr, 
> Error **errp);
>  static void migrate_hup_add(MigrationState *s, QIOChannel *ioc, GSourceFunc 
> cb,
>                              void *opaque)
>  {
> -        s->hup_source = qio_channel_create_watch(ioc, G_IO_HUP);
> -        g_source_set_callback(s->hup_source, cb, opaque, NULL);
> -        g_source_attach(s->hup_source, NULL);
> +    s->hup_source = qio_channel_create_watch(ioc, G_IO_HUP);
> +    g_source_set_callback(s->hup_source, cb,
> +                          QAPI_CLONE(MigrationAddress, opaque),
> +                          (GDestroyNotify)qapi_free_MigrationAddress);
> +    g_source_attach(s->hup_source, NULL);
>  }
>
>  static void migrate_hup_delete(MigrationState *s)
> @@ -2028,7 +2030,6 @@ static gboolean qmp_migrate_finish_cb(QIOChannel 
> *channel,
>      MigrationAddress *addr = opaque;
>
>      qmp_migrate_finish(addr, NULL);
> -    qapi_free_MigrationAddress(addr);
>      return G_SOURCE_REMOVE;
>  }
>
> @@ -2083,7 +2084,7 @@ void qmp_migrate(const char *uri, bool has_channels,
>       */
>      if (migrate_mode() == MIG_MODE_CPR_TRANSFER) {
>          migrate_hup_add(s, cpr_state_ioc(), 
> (GSourceFunc)qmp_migrate_finish_cb,
> -                        QAPI_CLONE(MigrationAddress, main_ch->addr));
> +                        main_ch->addr);
>
>      } else {
>          qmp_migrate_finish(main_ch->addr, errp);
> --

* Looks okay.
Reviewed-by: Prasad Pandit <[email protected]>

Thank you.
---
  - Prasad


Reply via email to