Re: [PATCH 2/2] hw/arm/xlnx-zynqmp.c: Add missing error-propagation code

[Philippe Mathieu-Daudé]

On 3/24/20 2:49 PM, Peter Maydell wrote:
> In some places in xlnx_zynqmp_realize() we were putting an
> error into our local Error*, but forgetting to check for
> failure and pass it back to the caller. Add the missing code.

This (incorrect) pattern is used in many places, so it might be easier 
to fix it once with a Coccinelle script...


i.e. a quick "git grep -3 object_property_set_uint":

hw/arm/bcm2835_peripherals.c:290
hw/arm/fsl-imx25.c:272
hw/arm/fsl-imx6.c:349
hw/i386/x86.c:110
hw/microblaze/xlnx-zynqmp-pmu.c:88
target/i386/cpu.c:5158

> Signed-off-by: Peter Maydell <address@hidden>
> ---
> Spotted while I was writing the previous patch.
> ---
>   hw/arm/xlnx-zynqmp.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index a13dbeeacec..b84d153d56a 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -530,8 +530,20 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>            * - eMMC Specification Version 4.51
>            */
>           object_property_set_uint(sdhci, 3, "sd-spec-version", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>           object_property_set_uint(sdhci, SDHCI_CAPABILITIES, "capareg", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>           object_property_set_uint(sdhci, UHS_I, "uhs", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>           object_property_set_bool(sdhci, true, "realized", &err);
>           if (err) {
>               error_propagate(errp, err);
> @@ -551,6 +563,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>           gchar *bus_name;
>   
>           object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>   
>           sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
>           sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
> @@ -565,6 +581,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>       }
>   
>       object_property_set_bool(OBJECT(&s->qspi), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
>       sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR);
>       sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR);
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]);
> @@ -619,6 +639,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>   
>       for (i = 0; i < XLNX_ZYNQMP_NUM_GDMA_CH; i++) {
>           object_property_set_uint(OBJECT(&s->gdma[i]), 128, "bus-width", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>           object_property_set_bool(OBJECT(&s->gdma[i]), true, "realized", &err);
>           if (err) {
>               error_propagate(errp, err);