On Fri, 3 Oct 2025 at 16:40, Peter Xu <[email protected]> wrote:
>
> From: Steve Sistare <[email protected]>
>
> Create the cpr-exec-command migration parameter, defined as a list of
> strings. It will be used for cpr-exec migration mode in a subsequent
> patch, and contains forward references to cpr-exec mode in the qapi
> doc.
>
> No functional change, except that cpr-exec-command is shown by the
> 'info migrate' command.
Hi; I was doing a 'git grep' for accidental misuses
of g_autofree on a char** (which will free only the
top level array, not the strings it points to), and
the usage in this commit confused me for a bit:
> + case MIGRATION_PARAMETER_CPR_EXEC_COMMAND: {
> + g_autofree char **strv = NULL;
g_shell_parse_argv() documents that we should free the
vector with g_strfreev(), but g_autofree will only
g_free() it. So this looks like a bug, but...
> + g_autoptr(GError) gerr = NULL;
> + strList **tail = &p->cpr_exec_command;
> +
> + if (!g_shell_parse_argv(valuestr, NULL, &strv, &gerr)) {
> + error_setg(&err, "%s", gerr->message);
> + break;
> + }
> + for (int i = 0; strv[i]; i++) {
> + QAPI_LIST_APPEND(tail, strv[i]);
...we copy out the pointers to the individual
strings here, and QAPI_LIST_APPEND() doesn't do a
string copy, so it's correct that we don't g_strfreev()
the char**.
Do you think it's worth a brief comment saying why
g_autofree is correct here, or is it obvious enough
without ?
> + }
> + p->has_cpr_exec_command = true;
> + break;
> + }
thanks
-- PMM
