Re: [PATCH 33/46] qom: Crash more nicely on object_property_get_link() failure

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Eric Blake <eblake@redhat.com> writes:
>
>> On 6/24/20 11:43 AM, Markus Armbruster wrote:
>>> Pass &error_abort instead of NULL where the returned value is
>>> dereferenced or asserted to be non-null.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>
>>> @@ -63,8 +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);
>>> -
>>> +    parent_mr = object_property_get_link(OBJECT(sbdev_mr), "container",
>>> +                                         &error_abort);
>>>       assert(parent_mr);
>>
>> Do we still need to keep the assert?
>
> Not really, I guess.
>
>>> +++ b/hw/ppc/spapr_pci_nvlink2.c
>>> @@ -141,9 +141,10 @@ static void spapr_phb_pci_collect_nvgpu(PCIBus *bus, 
>>> PCIDevice *pdev,
>>>       if (tgt) {
>>>           Error *local_err = NULL;
>>>           SpaprPhbPciNvGpuConfig *nvgpus = opaque;
>>> -        Object *mr_gpu = object_property_get_link(po, "nvlink2-mr[0]", 
>>> NULL);
>>> +        Object *mr_gpu = object_property_get_link(po, "nvlink2-mr[0]",
>>> +                                                  &error_abort);
>>>           Object *mr_npu = object_property_get_link(po, 
>>> "nvlink2-atsd-mr[0]",
>>> -                                                  NULL);
>>> +                                                  &error_abort);
>>>             g_assert(mr_gpu || mr_npu);
>>
>> Likewise.
>
> I'll drop both unless somebody objects.

The second hunk needs to be dropped instead: either of the two
object_property_get_link() may fail, just not both.