Daniel P. Berrangé <berra...@redhat.com> writes:
> On Wed, Oct 04, 2023 at 11:43:12AM -0300, Fabiano Rosas wrote:
>> Het Gala <het.g...@nutanix.com> writes:
>>
>> > This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri'
>> > string containing migration connection related information
>> > and stores them inside well defined 'MigrateAddress' struct.
>> >
>> > Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
>> > Signed-off-by: Het Gala <het.g...@nutanix.com>
>> > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
>> > ---
>> > migration/exec.c | 1 -
>> > migration/exec.h | 4 ++++
>> > migration/migration.c | 55 +++++++++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 59 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/migration/exec.c b/migration/exec.c
>> > index 2bf882bbe1..32f5143dfd 100644
>> > --- a/migration/exec.c
>> > +++ b/migration/exec.c
>> > @@ -27,7 +27,6 @@
>> > #include "qemu/cutils.h"
>> >
>> > #ifdef WIN32
>> > -const char *exec_get_cmd_path(void);
>> > const char *exec_get_cmd_path(void)
>> > {
>> > g_autofree char *detected_path = g_new(char, MAX_PATH);
>> > diff --git a/migration/exec.h b/migration/exec.h
>> > index b210ffde7a..736cd71028 100644
>> > --- a/migration/exec.h
>> > +++ b/migration/exec.h
>> > @@ -19,6 +19,10 @@
>> >
>> > #ifndef QEMU_MIGRATION_EXEC_H
>> > #define QEMU_MIGRATION_EXEC_H
>> > +
>> > +#ifdef WIN32
>> > +const char *exec_get_cmd_path(void);
>> > +#endif
>> > void exec_start_incoming_migration(const char *host_port, Error **errp);
>> >
>> > void exec_start_outgoing_migration(MigrationState *s, const char
>> > *host_port,
>> > diff --git a/migration/migration.c b/migration/migration.c
>> > index 6d3cf5d5cd..dcbd509d56 100644
>> > --- a/migration/migration.c
>> > +++ b/migration/migration.c
>> > @@ -65,6 +65,7 @@
>> > #include "sysemu/qtest.h"
>> > #include "options.h"
>> > #include "sysemu/dirtylimit.h"
>> > +#include "qemu/sockets.h"
>> >
>> > static NotifierList migration_state_notifiers =
>> > NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
>> > @@ -427,15 +428,64 @@ void migrate_add_address(SocketAddress *address)
>> > QAPI_CLONE(SocketAddress, address));
>> > }
>> >
>> > +static bool migrate_uri_parse(const char *uri,
>> > + MigrationAddress **channel,
>> > + Error **errp)
>> > +{
>> > + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>>
>> This cannot be g_autoptr because you're passing it out of scope at the
>> end of the function.
>
> It is still good to use g_autoptr to deal with the error paths.
>
> On the success path though you need g_steal_pointer(&addr) to
> prevent the autofree cleanup running.
Ah good point, this has been suggested in an earlier version already, I
forgot to mention. We should definitely use g_steal_pointer() whenever
the variable goes out of scope.
