qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v12 16/16] machine: Make smp_parse return a boolean


From: Daniel P . Berrangé
Subject: Re: [PATCH v12 16/16] machine: Make smp_parse return a boolean
Date: Fri, 1 Oct 2021 18:15:49 +0100
User-agent: Mutt/2.0.7 (2021-05-04)

On Fri, Oct 01, 2021 at 07:08:51PM +0200, Paolo Bonzini wrote:
> On 29/09/21 04:58, Yanan Wang wrote:
> > @@ -933,8 +935,7 @@ static void machine_set_smp(Object *obj, Visitor *v, 
> > const char *name,
> >           return;
> >       }
> > -    smp_parse(ms, config, errp);
> > -    if (*errp) {
> > +    if (!smp_parse(ms, config, errp)) {
> >           qapi_free_SMPConfiguration(config);
> >       }
> >   }
> > 
> 
> This is actually a leak, so I'm replacing this patch with

This patch isn't adding a leak, as there's no change in
control flow / exit paths.  AFAICT, the leak was introduced
in patch 15 instead, so the code below shoudl be squashed
into that, and this patch left as-is.

> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 54f04a5ac6..d49ebc24e2 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -897,7 +897,7 @@ static void machine_set_smp(Object *obj, Visitor *v, 
> const char *name,
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(obj);
>      MachineState *ms = MACHINE(obj);
> -    SMPConfiguration *config;
> +    g_autoptr(SMPConfiguration) config = NULL;
>      ERRP_GUARD();
>      if (!visit_type_SMPConfiguration(v, name, &config, errp)) {
> @@ -920,7 +920,7 @@ static void machine_set_smp(Object *obj, Visitor *v, 
> const char *name,
>      smp_parse(ms, config, errp);
>      if (*errp) {
> -        goto out_free;
> +        return;
>      }
>      /* sanity-check smp_cpus and max_cpus against mc */
> @@ -935,9 +935,6 @@ static void machine_set_smp(Object *obj, Visitor *v, 
> const char *name,
>                     ms->smp.max_cpus,
>                     mc->name, mc->max_cpus);
>      }
> -
> -out_free:
> -    qapi_free_SMPConfiguration(config);
>  }
>  static void machine_class_init(ObjectClass *oc, void *data)
> 
> which removes the need.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




reply via email to

[Prev in Thread] Current Thread [Next in Thread]