On Tue, 23 Dec 2025 at 14:32, Peter Xu <[email protected]> wrote:
>
> From: Fabiano Rosas <[email protected]>
>
> The MigrationState is a QOM object with TYPE_DEVICE as a parent. This
> was done about eight years ago so the migration code could make use of
> qdev properties to define the defaults for the migration parameters
> and to be able to expose migration knobs for debugging via the
> '-global migration' command line option.
>
> Due to unrelated historical reasons, three of the migration parameters
> (TLS options) received different types when used via the
> query-migrate-parameters QMP command than with the
> migrate-set-parameters command. This has created a lot of duplication
> in the migration code and in the QAPI documentation because the whole
> of MigrationParameters had to be duplicated as well.
>
> The migration code is now being fixed to remove the duplication and
> for that to happen the offending fields need to be reconciled into a
> single type. The StrOrNull type is going to be used.
>
> To keep the command line compatibility, the parameters need to
> continue being exposed via qdev properties accessible from the command
> line. Introduce a qdev property StrOrNull just for that.
>
> Note that this code is being kept in migration/options.c as this
> version of StrOrNull doesn't need to handle QNULL because it was never
> a valid option in the previous command line, which took a string.
>
> Signed-off-by: Fabiano Rosas <[email protected]>
> Acked-by: Peter Xu <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Peter Xu <[email protected]>
> ---
> migration/options.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
Hi; Coverity thinks there's a memory leak in this code
(CID 1643919). Now I know Coverity gets easily confused about
our visitor patterns, and I don't myself know the way they're
supposed to handle ownership of memory, so this should be
checked by somebody who does before adding any extra calls
to free. That said:
> +static void set_StrOrNull(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + const Property *prop = opaque;
> + StrOrNull **ptr = object_field_prop_ptr(obj, prop);
> + StrOrNull *str_or_null = g_new0(StrOrNull, 1);
> +
> + /*
> + * Only str to keep compatibility, QNULL was never used via
> + * command line.
> + */
> + str_or_null->type = QTYPE_QSTRING;
> + if (!visit_type_str(v, name, &str_or_null->u.s, errp)) {
> + return;
In this error-return code path, we don't seem to hand the
"str_or_null" pointer on to anybody, and we don't free it
either -- does this leak what we just allocated with g_new0() ?
> + }
> +
> + qapi_free_StrOrNull(*ptr);
> + *ptr = str_or_null;
> +}
thanks
-- PMM
