[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code |
Date: |
Thu, 18 Jan 2018 14:34:39 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 01/18/2018 09:55 AM, Philippe Mathieu-Daudé wrote:
> On Thu, Jan 18, 2018 at 9:03 AM, Paolo Bonzini <address@hidden> 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
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Fam Zheng, 2018/01/17
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Philippe Mathieu-Daudé, 2018/01/17
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Paolo Bonzini, 2018/01/18
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Philippe Mathieu-Daudé, 2018/01/18
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Paolo Bonzini, 2018/01/18
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Philippe Mathieu-Daudé, 2018/01/18
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Paolo Bonzini, 2018/01/18
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code,
Eric Blake <=
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Eduardo Habkost, 2018/01/18
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Paolo Bonzini, 2018/01/18
- Re: [Qemu-devel] [PATCH] scsi-generic: Simplify error handling code, Eduardo Habkost, 2018/01/18