Re: [PATCH v2 06/18] hw/acpi: Fix legacy CPU plug error API violations

[Michael S. Tsirkin]

On Wed, Dec 04, 2019 at 10:36:13AM +0100, Markus Armbruster wrote:
> legacy_acpi_cpu_plug_cb() dereferences @errp when
> acpi_set_cpu_present_bit() fails.  That's wrong; see the big comment
> in error.h.  Introduced in commit cc43364de7 "acpi/cpu-hotplug:
> introduce helper function to keep bit setting in one place".
> 
> No caller actually passes null, and acpi_set_cpu_present_bit() can't
> actually fail.
> 
> Fix anyway: drop acpi_set_cpu_present_bit()'s @errp parameter.
> 
> Cc: "Michael S. Tsirkin" <address@hidden>
> Cc: Igor Mammedov <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Igor Mammedov <address@hidden>

Reviewed-by: Michael S. Tsirkin <address@hidden>

Feel free to merge through your tree pls.

> ---
>  hw/acpi/cpu_hotplug.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index 3ac2045a95..9c3bcc84de 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -55,8 +55,7 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
>      },
>  };
>  
> -static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> -                                     Error **errp)
> +static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu)
>  {
>      CPUClass *k = CPU_GET_CLASS(cpu);
>      int64_t cpu_id;
> @@ -74,10 +73,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, 
> CPUState *cpu,
>  void legacy_acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
>                               AcpiCpuHotplug *g, DeviceState *dev, Error 
> **errp)
>  {
> -    acpi_set_cpu_present_bit(g, CPU(dev), errp);
> -    if (*errp != NULL) {
> -        return;
> -    }
> +    acpi_set_cpu_present_bit(g, CPU(dev));
>      acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
>  }
>  
> @@ -92,7 +88,7 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, 
> Object *owner,
>      gpe_cpu->device = owner;
>  
>      CPU_FOREACH(cpu) {
> -        acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> +        acpi_set_cpu_present_bit(gpe_cpu, cpu);
>      }
>  }
>  
> -- 
> 2.21.0