Re: [PATCH 34/46] qom: Don't handle impossible object_property_get_link() failure

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 6/24/20 6:43 PM, Markus Armbruster wrote:
>> Don't handle object_property_get_link() failure that can't happen
>> unless the programmer screwed up, pass &error_abort.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/arm/bcm2835_peripherals.c |  7 +------
>>  hw/arm/bcm2836.c             |  7 +------
>>  hw/display/bcm2835_fb.c      |  8 +-------
>>  hw/dma/bcm2835_dma.c         |  9 +--------
>>  hw/gpio/bcm2835_gpio.c       | 15 ++-------------
>>  hw/intc/nios2_iic.c          |  8 +-------
>>  hw/misc/bcm2835_mbox.c       |  9 +--------
>>  hw/misc/bcm2835_property.c   | 17 ++---------------
>>  hw/usb/hcd-dwc2.c            |  9 +--------
>>  9 files changed, 11 insertions(+), 78 deletions(-)
>> 
>> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
>> index 8313410ffe..3c40bda91e 100644
>> --- a/hw/arm/bcm2835_peripherals.c
>> +++ b/hw/arm/bcm2835_peripherals.c
>> @@ -134,12 +134,7 @@ static void bcm2835_peripherals_realize(DeviceState 
>> *dev, Error **errp)
>>      uint64_t ram_size, vcram_size;
>>      int n;
>>  
>> -    obj = object_property_get_link(OBJECT(dev), "ram", &err);
>> -    if (obj == NULL) {
>> -        error_setg(errp, "%s: required ram link not found: %s",
>> -                   __func__, error_get_pretty(err));
>> -        return;
>> -    }
>> +    obj = object_property_get_link(OBJECT(dev), "ram", &error_abort);
> [...]
>
> Should we now add an assert(errp) in object_property_get_link()?
> Basically this would force forks to adapt their code when
> rebasing.

Functions should not place additional restrictions @errp arguments
without a compelling reason.  What if you want genuinely don't need the
error details when object_property_get_link() fails?  Passing null is
better than passing &err only to error_free(err).

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!