Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code

[Eric Blake <eblake@redhat.com>]

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

On 01/18/2018 09:55 AM, Philippe Mathieu-Daudé wrote:
> On Thu, Jan 18, 2018 at 9:03 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 18/01/2018 12:21, Philippe Mathieu-Daudé wrote:
>>>> I'm not a fan of bool return types, in general (because "!" is often
>>>> success while "< 0" is failure) and especially when there is an Error**;
>>>> I disagree with commit 9d3b155186.  But the function is not in an area I
>>>> maintain so I'm queuing this, thanks.
>>> Do you prefer "if (local_err)" and "if (errp && *errp)" ?
>>
>> The latter is wrong.  I do prefer
> 
> Ok so my 253674981e24 missed that train too.

Markus has expressed as desire, as the error maintainer, to make errp
functions return a useful value for less boilerplate, and at one point
was even debating about Coccinelle scripts to make the conversion
easier.  Perhaps int with -1 is more reliable than bool for that useful
value, but this is definitely a topic of past discussion.

By the way, if (local_err) is definitely preferable; 'if (errp &&
*errp)' means that your behavior is different depending on whether the
caller wanted to ignore the error, and not whether you wanted to handle
the error.

> 
>>
>>     if (local_err) {
>>         error_propagate(errp, local_err);
>>         return;
>>     }

Yes, that's the right boilerplate if you don't have a return value witness.

>>
>> or maybe (but only if there is a meaning to a zero vs. positive return
>> value, or if errno is an important part of the returned Error *)
>>
>>     ret = f(..., errp);
>>     if (ret < 0) {
>>         return;
>>     }
>>
>>> I wondered once if a macro might improve this pattern but thought the
>>> code would get more obscure.
>>
>> Eduardo had a series to avoid error_propagate, where NULL was replaced
>> by a (non-NULL) IGNORED_ERRORS macro.  Then you could do:
>>
>>     f(..., errp);
>>     if (error_is_set(errp)) {
>>         return;
>>     }
>>
>> See here:
>> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03139.html
> 
> This series never hit /master!
> 
> Reading the thread I'm not sure what was the expected outcome.

And since Markus may not answer this thread for a while, I'm still not
sure if there is any expected outcome.

-- 
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 --]