Het Gala, Peter, or Fabiano, please review.
Steve Sistare <[email protected]> writes:
> Simplify the exec migration code by using list utility functions.
>
> As a side effect, this also fixes a minor memory leak. On function return,
> "g_auto(GStrv) argv" frees argv and each element, which is wrong, because
> the function does not own the individual elements. To compensate, the code
> uses g_steal_pointer which NULLs argv and prevents the destructor from
> running, but argv is leaked.
>
> Fixes: cbab4face57b ("migration: convert exec backend ...")
> Signed-off-by: Steve Sistare <[email protected]>
> ---
> migration/exec.c | 58
> ++++++++------------------------------------------------
> 1 file changed, 8 insertions(+), 50 deletions(-)
>
> diff --git a/migration/exec.c b/migration/exec.c
> index 47d2f3b..1312ca7 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -19,12 +19,12 @@
>
> #include "qemu/osdep.h"
> #include "qemu/error-report.h"
> +#include "qemu/strList.h"
> #include "channel.h"
> #include "exec.h"
> #include "migration.h"
> #include "io/channel-command.h"
> #include "trace.h"
> -#include "qemu/cutils.h"
>
> #ifdef WIN32
> const char *exec_get_cmd_path(void)
> @@ -39,51 +39,16 @@ const char *exec_get_cmd_path(void)
> }
> #endif
>
> -/* provides the length of strList */
> -static int
> -str_list_length(strList *list)
> -{
> - int len = 0;
> - strList *elem;
> -
> - for (elem = list; elem != NULL; elem = elem->next) {
> - len++;
> - }
> -
> - return len;
> -}
> -
> -static void
> -init_exec_array(strList *command, char **argv, Error **errp)
> -{
> - int i = 0;
> - strList *lst;
> -
> - for (lst = command; lst; lst = lst->next) {
> - argv[i++] = lst->value;
> - }
> -
> - argv[i] = NULL;
> - return;
> -}
> -
> void exec_start_outgoing_migration(MigrationState *s, strList *command,
> Error **errp)
> {
> - QIOChannel *ioc;
> -
> - int length = str_list_length(command);
> - g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1);
> -
> - init_exec_array(command, argv, errp);
> + QIOChannel *ioc = NULL;
> + g_auto(GStrv) argv = strv_from_strList(command);
> + const char * const *args = (const char * const *) argv;
> g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
>
> trace_migration_exec_outgoing(new_command);
> - ioc = QIO_CHANNEL(
> - qio_channel_command_new_spawn(
> - (const char * const *) g_steal_pointer(&argv),
> - O_RDWR,
> - errp));
> + ioc = QIO_CHANNEL(qio_channel_command_new_spawn(args, O_RDWR, errp));
> if (!ioc) {
> return;
> }
> @@ -105,19 +70,12 @@ static gboolean
> exec_accept_incoming_migration(QIOChannel *ioc,
> void exec_start_incoming_migration(strList *command, Error **errp)
> {
> QIOChannel *ioc;
> -
> - int length = str_list_length(command);
> - g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1);
> -
> - init_exec_array(command, argv, errp);
> + g_auto(GStrv) argv = strv_from_strList(command);
> + const char * const *args = (const char * const *) argv;
> g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
>
> trace_migration_exec_incoming(new_command);
> - ioc = QIO_CHANNEL(
> - qio_channel_command_new_spawn(
> - (const char * const *) g_steal_pointer(&argv),
> - O_RDWR,
> - errp));
> + ioc = QIO_CHANNEL(qio_channel_command_new_spawn(args, O_RDWR, errp));
> if (!ioc) {
> return;
> }
