On Thu, Jan 22, 2026 at 08:24:56PM -0300, Fabiano Rosas wrote:
> Fix a couple of leaks detected by Coverity. Both are currently
> harmless because the visitor in the setter can never fail and the
> whole of the getter is unused, it's only purpose at the moment is to
> provide a complete implementation of the StrOrNull property.
>
> Fixes: CID 1643919
> Fixes: CID 1643920
> Reported-by: Peter Maydell <[email protected]>
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2280325023
> ---
> migration/options.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/migration/options.c b/migration/options.c
> index 9a5a39c886..9dc44a3736 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -225,6 +225,7 @@ static void get_StrOrNull(Object *obj, Visitor *v, const
> char *name,
> str_or_null = g_new0(StrOrNull, 1);
> str_or_null->type = QTYPE_QSTRING;
> str_or_null->u.s = g_strdup("");
> + *ptr = str_or_null;
Yep, fixes the leak.
However does it then mean a get() can internally update this Property? It
used to be NULL, now it's QTYPE_STRING "".
It may make slightly more sense to me to have zero side effect on a get()
call. Say, keeping *ptr==NULL after get() returns (as before), while we can
use a temp string ("") for the visitor.
I randomly checked another get() provider, e.g. get_drive() does that.
Should we follow?
> } else {
> /* the setter doesn't allow QNULL */
> assert(str_or_null->type != QTYPE_QNULL);
> @@ -245,6 +246,7 @@ static void set_StrOrNull(Object *obj, Visitor *v, const
> char *name,
> */
> str_or_null->type = QTYPE_QSTRING;
> if (!visit_type_str(v, name, &str_or_null->u.s, errp)) {
> + qapi_free_StrOrNull(str_or_null);
This looks correct. Or, simpler way is we only do g_new0() after the
visit_type_str() successfully returned (and pass a temp char* into it).
> return;
> }
>
> --
> 2.51.0
>
--
Peter Xu
