Re: [Qemu-devel] [PATCH 0/9] Miscellaneous error reporting improvements

[Eric Blake <eblake@redhat.com>]

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

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


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