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

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 05/29/2015 05:22 AM, Markus Armbruster wrote:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>>> Am 28.05.2015 um 14:21 hat Markus Armbruster geschrieben:
>>>> Touches vl.c, which gives me pretext to ask Paolo: would you be
>>>> willing to take this through your tree?  Or should I take it through
>>>> mine?
>>>
>>> After this series we have an ugly half-converted state where
>>> qemu_opts_foreach() has both a return code and an Error object,
>>> and it's not generally true that an error is set for a failing
>>> return code.
>>>
>>> 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.

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.

To keep things simple, I lean towards rejecting R3 and passing NULL.

Opinions?