| Message ID | 20200324134947.15384-3-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/xlnx-zynqmp.c: fix some error-handling code | expand |
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 <peter.maydell@linaro.org> > --- > 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); >
On Tue, Mar 24, 2020 at 6:50 AM Peter Maydell <peter.maydell@linaro.org> 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. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > 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); > -- > 2.20.1 > >
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);
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. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Spotted while I was writing the previous patch. --- hw/arm/xlnx-zynqmp.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)