Re: [PATCH] migration/options: Fix leaks in StrOrNull accessors

[Peter Xu <peterx@redhat.com>]

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 <peter.maydell@linaro.org>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> 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