All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org
Cc: Andrew Jeffery <andrew@aj.id.au>,
	Peter Maydell <peter.maydell@linaro.org>,
	qemu-arm@nongnu.org, Joel Stanley <joel@jms.id.au>
Subject: Re: [PATCH 17/22] hw/arm: Drop useless object_property_set_link() error handling
Date: Mon, 22 Jun 2020 14:37:50 +0200	[thread overview]
Message-ID: <0cad765e-879c-973f-ea87-c691b410625d@kaod.org> (raw)
In-Reply-To: <20200622104250.1404835-18-armbru@redhat.com>

On 6/22/20 12:42 PM, Markus Armbruster wrote:
> object_property_set_link() fails when the property doesn't exist, is
> not settable, or its .check() method fails.  These are all programming
> errors here, so passing it &error_abort is appropriate.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: "Cédric Le Goater" <clg@kaod.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

For the Aspeed parts,
 
Reviewed-by: Cédric Le Goater <clg@kaod.org>


> ---
>  hw/arm/armsse.c         | 53 ++++++++++-------------------------------
>  hw/arm/armv7m.c         |  7 ++----
>  hw/arm/aspeed_ast2600.c | 20 ++++------------
>  hw/arm/aspeed_soc.c     | 14 ++++-------
>  hw/arm/nrf51_soc.c      |  6 +----
>  5 files changed, 24 insertions(+), 76 deletions(-)
> 
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index e8f8f60abc..c73cc6badf 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -564,16 +564,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>                                                  &s->container, -1);
>          }
>          object_property_set_link(cpuobj, OBJECT(&s->cpu_container[i]),
> -                                 "memory", &err);
> -        if (err) {
> -            error_propagate(errp, err);
> -            return;
> -        }
> -        object_property_set_link(cpuobj, OBJECT(s), "idau", &err);
> -        if (err) {
> -            error_propagate(errp, err);
> -            return;
> -        }
> +                                 "memory", &error_abort);
> +        object_property_set_link(cpuobj, OBJECT(s), "idau", &error_abort);
>          sysbus_realize(SYS_BUS_DEVICE(cpuobj), &err);
>          if (err) {
>              error_propagate(errp, err);
> @@ -700,11 +692,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>          object_property_set_link(OBJECT(&s->mpc[i]), OBJECT(&s->sram[i]),
> -                                 "downstream", &err);
> -        if (err) {
> -            error_propagate(errp, err);
> -            return;
> -        }
> +                                 "downstream", &error_abort);
>          sysbus_realize(SYS_BUS_DEVICE(&s->mpc[i]), &err);
>          if (err) {
>              error_propagate(errp, err);
> @@ -755,11 +743,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer0), 0,
>                         armsse_get_common_irq_in(s, 3));
>      mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->timer0), 0);
> -    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[0]", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[0]",
> +                             &error_abort);
>  
>      qdev_prop_set_uint32(DEVICE(&s->timer1), "pclk-frq", s->mainclk_frq);
>      sysbus_realize(SYS_BUS_DEVICE(&s->timer1), &err);
> @@ -770,12 +755,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer1), 0,
>                         armsse_get_common_irq_in(s, 4));
>      mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->timer1), 0);
> -    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[1]", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> -
> +    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[1]",
> +                             &error_abort);
>  
>      qdev_prop_set_uint32(DEVICE(&s->dualtimer), "pclk-frq", s->mainclk_frq);
>      sysbus_realize(SYS_BUS_DEVICE(&s->dualtimer), &err);
> @@ -786,11 +767,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->dualtimer), 0,
>                         armsse_get_common_irq_in(s, 5));
>      mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->dualtimer), 0);
> -    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[2]", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[2]",
> +                             &error_abort);
>  
>      if (info->has_mhus) {
>          /*
> @@ -815,12 +793,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>              port = g_strdup_printf("port[%d]", i + 3);
>              mr = sysbus_mmio_get_region(mhu_sbd, 0);
>              object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr),
> -                                     port, &err);
> +                                     port, &error_abort);
>              g_free(port);
> -            if (err) {
> -                error_propagate(errp, err);
> -                return;
> -            }
>  
>              /*
>               * Each MHU has an irq line for each CPU:
> @@ -967,11 +941,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->s32ktimer), 0,
>                         armsse_get_common_irq_in(s, 2));
>      mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->s32ktimer), 0);
> -    object_property_set_link(OBJECT(&s->apb_ppc1), OBJECT(mr), "port[0]", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->apb_ppc1), OBJECT(mr), "port[0]",
> +                             &error_abort);
>  
>      sysbus_realize(SYS_BUS_DEVICE(&s->apb_ppc1), &err);
>      if (err) {
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index ce83586e03..3308211e9c 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -170,11 +170,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
>      object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
>                               &error_abort);
>      if (object_property_find(OBJECT(s->cpu), "idau", NULL)) {
> -        object_property_set_link(OBJECT(s->cpu), s->idau, "idau", &err);
> -        if (err != NULL) {
> -            error_propagate(errp, err);
> -            return;
> -        }
> +        object_property_set_link(OBJECT(s->cpu), s->idau, "idau",
> +                                 &error_abort);
>      }
>      if (object_property_find(OBJECT(s->cpu), "init-svtor", NULL)) {
>          object_property_set_uint(OBJECT(s->cpu), s->init_svtor,
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 08b3592e36..4efac02e2b 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -341,11 +341,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>      }
>  
>      /* I2C */
> -    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram",
> +                             &error_abort);
>      sysbus_realize(SYS_BUS_DEVICE(&s->i2c), &err);
>      if (err) {
>          error_propagate(errp, err);
> @@ -363,11 +360,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>      }
>  
>      /* FMC, The number of CS is set at the board level */
> -    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram",
> +                             &error_abort);
>      object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
>                              "sdram-base", &err);
>      if (err) {
> @@ -388,11 +382,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>      /* SPI */
>      for (i = 0; i < sc->spis_num; i++) {
>          object_property_set_link(OBJECT(&s->spi[i]), OBJECT(s->dram_mr),
> -                                 "dram", &err);
> -        if (err) {
> -            error_propagate(errp, err);
> -            return;
> -        }
> +                                 "dram", &error_abort);
>          object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
>          sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
>          if (err) {
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index ec21de50ce..03b91bade6 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -300,11 +300,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>      }
>  
>      /* I2C */
> -    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram",
> +                             &error_abort);
>      sysbus_realize(SYS_BUS_DEVICE(&s->i2c), &err);
>      if (err) {
>          error_propagate(errp, err);
> @@ -315,11 +312,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>                         aspeed_soc_get_irq(s, ASPEED_I2C));
>  
>      /* FMC, The number of CS is set at the board level */
> -    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram",
> +                             &error_abort);
>      object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
>                              "sdram-base", &err);
>      if (err) {
> diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
> index 5a8961ddbb..20dd8b5897 100644
> --- a/hw/arm/nrf51_soc.c
> +++ b/hw/arm/nrf51_soc.c
> @@ -66,11 +66,7 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
>      }
>  
>      object_property_set_link(OBJECT(&s->cpu), OBJECT(&s->container), "memory",
> -            &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +                             &error_abort);
>      sysbus_realize(SYS_BUS_DEVICE(&s->cpu), &err);
>      if (err) {
>          error_propagate(errp, err);
> 



  reply	other threads:[~2020-06-22 12:38 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-22 10:42 [PATCH 00/22] Error handling fixes & cleanups Markus Armbruster
2020-06-22 10:42 ` [PATCH 01/22] net/virtio: Fix failover_replug_primary() return value regression Markus Armbruster
2020-06-22 21:25   ` Jens Freimann
2020-06-24 15:12     ` Michael S. Tsirkin
2020-06-22 10:42 ` [PATCH 02/22] pci: Delete useless error_propagate() Markus Armbruster
2020-06-22 21:26   ` Jens Freimann
2020-06-24 15:13     ` Michael S. Tsirkin
2020-06-22 10:42 ` [PATCH 03/22] Clean up some calls to ignore Error objects the right way Markus Armbruster
2020-06-22 14:43   ` Greg Kurz
2020-06-22 10:42 ` [PATCH 04/22] tests: Use &error_abort where appropriate Markus Armbruster
2020-06-22 11:26   ` Thomas Huth
2020-06-22 10:42 ` [PATCH 05/22] tests: Use error_free_or_abort() " Markus Armbruster
2020-06-22 10:42 ` [PATCH 06/22] usb/dev-mtp: Fix Error double free after inotify failure Markus Armbruster
2020-06-22 10:42 ` [PATCH 07/22] spapr: Plug minor memory leak in spapr_machine_init() Markus Armbruster
2020-06-22 14:54   ` Greg Kurz
2020-06-22 10:42 ` [PATCH 08/22] qga: Plug unlikely memory leak in guest-set-memory-blocks Markus Armbruster
2020-06-23  8:35   ` Zhanghailiang
2020-06-22 10:42 ` [PATCH 09/22] sd/milkymist-memcard: Plug minor memory leak in realize Markus Armbruster
2020-06-22 10:42 ` [PATCH 10/22] test-util-filemonitor: Plug unlikely memory leak Markus Armbruster
2020-06-22 10:42 ` [PATCH 11/22] vnc: Plug minor memory leak in vnc_display_open() Markus Armbruster
2020-06-22 10:42 ` [PATCH 12/22] tests/qom-proplist: Delete a superfluous error_free() Markus Armbruster
2020-06-22 10:42 ` [PATCH 13/22] aspeed: Clean up roundabout error propagation Markus Armbruster
2020-06-22 12:37   ` Cédric Le Goater
2020-06-22 10:42 ` [PATCH 14/22] qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp Markus Armbruster
2020-06-22 10:42 ` [PATCH 15/22] qdev: Drop qbus_set_hotplug_handler() " Markus Armbruster
2020-06-22 10:42 ` [PATCH 16/22] hw: Fix error API violation around object_property_set_link() Markus Armbruster
2020-06-22 12:47   ` Auger Eric
2020-06-22 21:22   ` Alistair Francis
2020-06-22 10:42 ` [PATCH 17/22] hw/arm: Drop useless object_property_set_link() error handling Markus Armbruster
2020-06-22 12:37   ` Cédric Le Goater [this message]
2020-06-22 10:42 ` [PATCH 18/22] riscv/sifive_u: Fix sifive_u_soc_realize() error API violations Markus Armbruster
2020-06-22 10:42   ` Markus Armbruster
2020-06-22 21:23   ` Alistair Francis
2020-06-22 21:23     ` Alistair Francis
2020-06-22 10:42 ` [PATCH 19/22] riscv_hart: Fix riscv_harts_realize() " Markus Armbruster
2020-06-22 10:42   ` Markus Armbruster
2020-06-22 21:19   ` Alistair Francis
2020-06-22 21:19     ` Alistair Francis
2020-06-22 10:42 ` [PATCH 20/22] mips/cps: Fix mips_cps_realize() " Markus Armbruster
2020-06-22 10:42 ` [PATCH 21/22] x86: Fix x86_cpu_new() " Markus Armbruster
2020-06-22 10:42 ` [PATCH 22/22] amd_iommu: Fix amdvi_realize() error API violation Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0cad765e-879c-973f-ea87-c691b410625d@kaod.org \
    --to=clg@kaod.org \
    --cc=andrew@aj.id.au \
    --cc=armbru@redhat.com \
    --cc=joel@jms.id.au \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.