Re: [PATCH-for-5.0 05/12] hw/i386/x86: Add missing error-propagation code

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 25 Mar 2020 at 19:18, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Patch created mechanically by running:
>
>   $ spatch \
>     --macro-file scripts/cocci-macro-file.h --include-headers \
>     --sp-file scripts/coccinelle/object_property_missing_error_propagate.cocci \
>     --keep-comments --smpl-spacing --in-place --dir hw
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/i386/x86.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 87b73fe33c..0a4865d4a9 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -103,13 +103,17 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
>  void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
>  {
>      Object *cpu = NULL;
>      Error *local_err = NULL;
>
>      cpu = object_new(MACHINE(x86ms)->cpu_type);
>
>      object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
>      object_property_set_bool(cpu, true, "realized", &local_err);
>
>      object_unref(cpu);
>      error_propagate(errp, local_err);
>  }

Hmm, not sure about this one -- in the error-exit path
for failure-to-realize we object_unref(), do we need to
do so also if we error-exit for failure to set the apic-id ?

(Since apic-id is a PROP_UINT32 with no set function, there
is not currently any way for the set_uint to fail, as it
happens. But we ought to either have the error-propagation
right or use error_abort if we truly believe it can never fail.)

thanks
-- PMM