[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 23/44] qom: Crash more nicely on object_property_get_link(
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 23/44] qom: Crash more nicely on object_property_get_link() failure |
Date: |
Sat, 04 Jul 2020 18:15:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> 02.07.2020 18:49, Markus Armbruster wrote:
>> Pass &error_abort instead of NULL where the returned value is
>> dereferenced or asserted to be non-null. Drop a now redundant
>> assertion.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> hw/core/platform-bus.c | 6 +++---
>> hw/ppc/spapr_drc.c | 3 ++-
>> hw/ppc/spapr_hcall.c | 3 ++-
>> hw/ppc/spapr_pci_nvlink2.c | 3 ++-
>> ui/vnc.c | 2 +-
>> 5 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/core/platform-bus.c b/hw/core/platform-bus.c
>> index d494e5cec1..5037ca265e 100644
>> --- a/hw/core/platform-bus.c
>> +++ b/hw/core/platform-bus.c
>> @@ -22,6 +22,7 @@
>> #include "qemu/osdep.h"
>> #include "hw/platform-bus.h"
>> #include "hw/qdev-properties.h"
>> +#include "qapi/error.h"
>> #include "qemu/error-report.h"
>> #include "qemu/module.h"
>> @@ -63,9 +64,8 @@ hwaddr
>> platform_bus_get_mmio_addr(PlatformBusDevice *pbus, SysBusDevice
>> *sbdev,
>> return -1;
>> }
>> - parent_mr = object_property_get_link(OBJECT(sbdev_mr),
>> "container", NULL);
>> -
>> - assert(parent_mr);
>> + parent_mr = object_property_get_link(OBJECT(sbdev_mr), "container",
>> + &error_abort);
>> if (parent_mr != pbus_mr_obj) {
>> /* MMIO region is not mapped on platform bus */
>> return -1;
>> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
>> index d10193f39e..1f18b79348 100644
>> --- a/hw/ppc/spapr_drc.c
>> +++ b/hw/ppc/spapr_drc.c
>> @@ -870,7 +870,8 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner,
>> uint32_t drc_type_mask)
>> continue;
>> }
>> - obj = object_property_get_link(root_container,
>> prop->name, NULL);
>> + obj = object_property_get_link(root_container, prop->name,
>> + &error_abort);
>> drc = SPAPR_DR_CONNECTOR(obj);
>> drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 0f54988f2e..c1d01228c6 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1655,7 +1655,8 @@ static void
>> spapr_handle_transient_dev_before_cas(SpaprMachineState *spapr)
>> continue;
>> }
>> drc = SPAPR_DR_CONNECTOR(object_property_get_link(drc_container,
>> - prop->name,
>> NULL));
>> + prop->name,
>> + &error_abort));
>> if (spapr_drc_transient(drc)) {
>> spapr_drc_reset(drc);
>> diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c
>> index 8332d5694e..dd8cd6db96 100644
>> --- a/hw/ppc/spapr_pci_nvlink2.c
>> +++ b/hw/ppc/spapr_pci_nvlink2.c
>> @@ -358,7 +358,8 @@ void spapr_phb_nvgpu_ram_populate_dt(SpaprPhbState
>> *sphb, void *fdt)
>> for (i = 0; i < sphb->nvgpus->num; ++i) {
>> SpaprPhbPciNvGpuSlot *nvslot = &sphb->nvgpus->slots[i];
>> Object *nv_mrobj = object_property_get_link(OBJECT(nvslot->gpdev),
>> - "nvlink2-mr[0]", NULL);
>> + "nvlink2-mr[0]",
>> + &error_abort);
>> uint32_t associativity[] = {
>> cpu_to_be32(0x4),
>> SPAPR_GPU_NUMA_ID,
>
> the following
> uint64_t size = object_property_get_uint(nv_mrobj, "size", NULL);
>
> is a good candidate for error_abort as well.
Mission creep :)
We have many, many object_property_get_FOO() and _set_FOO() calls that
ignore errors. Probably most of them should abort on error instead.
Let's leave that for a future series.
> with it or not:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Thanks!
- [PATCH v2 43/44] qdev: Ignore Error objects where the return value suffices, (continued)
- [PATCH v2 43/44] qdev: Ignore Error objects where the return value suffices, Markus Armbruster, 2020/07/02
- [PATCH v2 41/44] error: Avoid error_propagate() after migrate_add_blocker(), Markus Armbruster, 2020/07/02
- [PATCH v2 12/44] qemu-option: Make functions taking Error ** return bool, not void, Markus Armbruster, 2020/07/02
- [PATCH v2 14/44] block: Avoid error accumulation in bdrv_img_create(), Markus Armbruster, 2020/07/02
- [PATCH v2 23/44] qom: Crash more nicely on object_property_get_link() failure, Markus Armbruster, 2020/07/02
- [PATCH v2 32/44] qdev: Use returned bool to check for failure, Coccinelle part, Markus Armbruster, 2020/07/02
- [PATCH v2 34/44] error: Eliminate error_propagate() with Coccinelle, part 1, Markus Armbruster, 2020/07/02
- [PATCH v2 26/44] qom: Put name parameter before value / visitor parameter, Markus Armbruster, 2020/07/02
- [PATCH v2 15/44] hmp: Eliminate a variable in hmp_migrate_set_parameter(), Markus Armbruster, 2020/07/02