On Mon, Dec 23, 2024 at 11:11:46PM -0800, Yichen Wang wrote:
> > @@ -563,6 +572,15 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> > QDict *qdict)
> > p->has_x_checkpoint_delay = true;
> > visit_type_uint32(v, param, &p->x_checkpoint_delay, &err);
> > break;
> > + case MIGRATION_PARAMETER_ACCEL_PATH:
> > + p->has_accel_path = true;
> > + char **strv = g_strsplit(valuestr ? : "", " ", -1);
> > + strList **tail = &p->accel_path;
> > + for (int i = 0; strv[i]; i++) {
> > + QAPI_LIST_APPEND(tail, strv[i]);
> > + }
> > + g_strfreev(strv);
> > + break;
>
> I am doing my final testing, and seeing a new issue for above. This
> code doesn't really work, because strv is freed and all contents after
> the string split are gone. So here is what I am thinking:
>
> 1. This is supposed to be an easy visit_type_strList(v, param,
> &p->accel_path, &err), but it actually doesn't work. The code will
> throw:
> qemu-system-x86_64.dsa: ../../../qapi/string-input-visitor.c:343:
> parse_type_str: Assertion `siv->lm == LM_NONE' failed.
> when you are doing "migrate_set_parameter accel-path
> dsa:/dev/dsa/wq0.1" from HMP.
IIUC that's for JSON only.
>
> 2. If I remove the g_strfreev(strv), things are working perfectly. But
> I am worried about the memory leak here. As technically if you keep
> doing migrate_set_parameter for say 1 million times, memory will be
> exhausted.
Right, better not leak mem. Can you dup the str when constructing the
artifact? I mean something like:
- QAPI_LIST_APPEND(tail, strv[i]);
+ QAPI_LIST_APPEND(tail, g_strdup(strv[i]));
Thanks,
--
Peter Xu
