Re: [Qemu-devel] [PATCH v2 1/6] hw/ide: Convert DeviceClass init to realize

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1695 bytes --]

On 09/15/2017 04:35 PM, John Snow wrote:
> 
> 
> On 08/04/2017 06:26 AM, Mao Zhongyi wrote:
>> Replace init with realize in IDEDeviceClass, which has errp
>> as a parameter. So all the implementations now use error_setg
>> instead of error_report for reporting error.
>>

>> @@ -2398,7 +2399,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
>>                     const char *version, const char *serial, const char *model,
>>                     uint64_t wwn,
>>                     uint32_t cylinders, uint32_t heads, uint32_t secs,
>> -                   int chs_trans)
>> +                   int chs_trans, Error **errp)
> 
> this function now requires an additional invariant, which is that we
> must return -1 AND set errp. Probably wisest to just get rid of the
> return code so that we don't accidentally goof this up in the future.
> 
> I think Markus has had some guidance on this in the past, but admittedly
> I can't remember his preference.

Returning void requires callers to use boilerplate:

bar(..., Error **errp)
{
    Error *err = NULL;
    foo(..., &err);
    if (err) {
        error_propagate(errp, err);
        handle error ...;
        return;
    }
}

whereas keeping the invariant that a -1 return is synonymous with err
being set is simpler:

bar(..., Error **errp)
{
    if (foo(..., errp) < 0) {
        handle error ...;
        return;
    }
}

So these days, the preference is to KEEP the redundancy rather than
eliminate it, due to ease-of-use concerns.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]