[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/9] Miscellaneous error reporting improvements
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 0/9] Miscellaneous error reporting improvements |
Date: |
Tue, 02 Jun 2015 06:51:01 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 06/02/2015 05:51 AM, Markus Armbruster wrote:
>>>> The most confusing part about this is that you have &error_abort almost
>>>> everywhere, but the function doesn't actually abort on error, but rather
>>>> returns a negative error code and leaves errp alone.
>>>
>>> True. The function contract spells it out, which hopefully reduces the
>>> confusion somewhat.
>>
>> Except that you don't enforce the contract; I suggested adding
>> assert(!*errp) at the right place in the two conversions.
>>
>>>
>>> Would you find NULL less confusing than &error_abort?
>>
>> NULL says to ignore errors, &error_abort says to diagnose errors as
>> programming bugs. If we know we aren't going to have an error, I prefer
>> diagnosing coding bugs.
>
> You prefer &error_abort, Kevin prefers NULL, so I need to figure out
> what I prefer to break the tie :)
>
> I think we can agree on these two rules on Error ** arguments:
>
> R1: When caller doesn't care whether the callee sets an error, it should
> pass NULL.
>
> R2: When a caller relies on the callee not setting an error, it should
> pass &error_abort.
Yes, these two rules cover the current state of the art.
>
> R1 applies, R2 does not, thus we should pass NULL.
>
> The case for &error_abort requires a third rule:
>
> Proposed R3: When a caller knows that the callee won't set an error, it
> may pass &error_abort to document this knowledge even when it doesn't
> actually rely on it (thus R2 doesn't apply). This is an exception to
> R1.
Or, as I explored in another message, if the caller passes NULL, but we
then turn it to &error_abort locally, to enforce that the callback does
not set an error for either success or failure.
>
> To keep things simple, I lean towards rejecting R3 and passing NULL.
>
> Opinions?
At this point I'm leaning towards simplicity - pass NULL, and not worth
modifying the contract (passing NULL does not need to get transformed
into error_abort).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature