On Mon, Jun 22, 2020 at 3:51 AM Markus Armbruster <[email protected]> wrote:
>
> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> pointer to a variable containing NULL.  Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
>
> riscv_harts_realize() is wrong that way: it passes @errp to
> riscv_hart_realize() in a loop.  I can't tell offhand whether this can
> fail.
>
> Fix by checking for failure in each iteration.
>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Alistair Francis <[email protected]>
> Cc: Sagar Karandikar <[email protected]>
> Cc: Bastian Koppelmann <[email protected]>
> Cc: Bin Meng <[email protected]>
> Cc: [email protected]
> Signed-off-by: Markus Armbruster <[email protected]>

Reviewed-by: Alistair Francis <[email protected]>

Alistair

> ---
>  hw/riscv/riscv_hart.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index e26c382259..f59fe52f0f 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -40,19 +40,13 @@ static void riscv_harts_cpu_reset(void *opaque)
>      cpu_reset(CPU(cpu));
>  }
>
> -static void riscv_hart_realize(RISCVHartArrayState *s, int idx,
> +static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
>                                 char *cpu_type, Error **errp)
>  {
> -    Error *err = NULL;
> -
>      object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
>      s->harts[idx].env.mhartid = s->hartid_base + idx;
>      qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
> -    qdev_realize(DEVICE(&s->harts[idx]), NULL, &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
>  }
>
>  static void riscv_harts_realize(DeviceState *dev, Error **errp)
> @@ -63,7 +57,9 @@ static void riscv_harts_realize(DeviceState *dev, Error 
> **errp)
>      s->harts = g_new0(RISCVCPU, s->num_harts);
>
>      for (n = 0; n < s->num_harts; n++) {
> -        riscv_hart_realize(s, n, s->cpu_type, errp);
> +        if (!riscv_hart_realize(s, n, s->cpu_type, errp)) {
> +            return;
> +        }
>      }
>  }
>
> --
> 2.26.2
>
>

Reply via email to