* 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. 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 |_______/
