Re: [PATCH] migration/cpr: Document obscure usage of g_autofree when parse str

["Dr. David Alan Gilbert" <dave@treblig.org>]

* Peter Xu (peterx@redhat.com) wrote:
> HMP parsing of cpr_exec_command contains an obscure usage of g_autofree.
> Provide a document for it to be clear that it's intentional, rather than
> memory leaked.
> 
> Cc: Dr. David Alan Gilbert <dave@treblig.org>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration-hmp-cmds.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 847d18faaa..79426bf5d7 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -734,6 +734,12 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>          visit_type_bool(v, param, &p->direct_io, &err);
>          break;
>      case MIGRATION_PARAMETER_CPR_EXEC_COMMAND: {
> +        /*
> +         * NOTE: g_autofree will only auto g_free() the strv array when
> +         * needed, it will not free the strings within the array. It's
> +         * intentional: when strv is set, the ownership of the strings will
> +         * always be passed to p->cpr_exec_command via QAPI_LIST_APPEND().
> +         */

Eww that's a bit weird isn't it.
It's not clear to me if g_shell_parse_argv() might return an error part
way through its parsing, and if it does whether there may be valid entries in
strv which really do need freeing.

https://docs.gtk.org/glib/func.shell_parse_argv.html doesn't seem to say.

Dave

>          g_autofree char **strv = NULL;
>          g_autoptr(GError) gerr = NULL;
>          strList **tail = &p->cpr_exec_command;
> -- 
> 2.50.1
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/