* Peter Xu ([email protected]) wrote: > On Thu, Oct 23, 2025 at 05:42:27PM +0000, Dr. David Alan Gilbert wrote: > > * Peter Xu ([email protected]) 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 <[email protected]> > > > Reported-by: Peter Maydell <[email protected]> > > > Signed-off-by: Peter Xu <[email protected]> > > > --- > > > 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. > > I checked glib code, it should be fine, the code looks like this since 2000: > > gboolean > g_shell_parse_argv (const gchar *command_line, > gint *argcp, > gchar ***argvp, > GError **error) > { > ... > failed: > > g_assert (error == NULL || *error != NULL); > g_strfreev (argv); <------------- > g_slist_free_full (tokens, g_free); > > return FALSE; > }
OK, thanks for checking. > The hope is with the comment, the current way is still the most efficient, > avoiding strdup()s. Yeh, it's at least better with the comment! Reviewed-by: Dr. David Alan Gilbert <[email protected]> > Still, let me know if any of us still prefer changing the code instead of > adding the comment.. The perf isn't a major issue, afaiu. But it's still > good to consider that always, I believe that was when Steve developed this. > > Thanks! Thanks for adding it. Dave > -- > Peter Xu > -- -----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 |_______/
