Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> On Fri, Jun 30, 2017 at 01:40:58PM +0200, Markus Armbruster wrote:
> [...]
>> 
>> I doubt the macros make the bug fixing materially easier, and I doubt
>> they can reduce future bugs of this kind.  What they can do is letting
>> us get rid of error_propagate() boilerplate with relative ease.
>> 
>> If we switch to returning success/failure (which also gets rid of the
>> boilerplate), then the macros may still let us get rid of boilerplate
>> more quickly, for some additional churn.  Worthwhile?  Depends on how
>> long the return value change takes us.
>> 
>> I think the first order of business is to figure out whether we want to
>> pursue returning success/failure.
>
> About this, I'm unsure.  Returning error information in two separate
> locations (the return value and *errp) makes it easier to introduce bugs
> that are hard to detect.

I sympathize with this argument.  It's exactly what that made us avoid
returning a success/failure indication.

Except when we don't actually avoid it:

* Functions returning a pointer typically return non-null on success,
  null on failure.  Has inconsistency between return value and Error
  setting been a problem in practice?  Nope.

* Quite a few functions return 0 on success, -errno on failure, to let
  callers handle different errors differently.  Has inconsistency been a
  problem in practice?  Nope again.

  Aside: the original Error plan was to have callers check ErrorClass,
  but that didn't work out.

* Functions with a side effect typically either do their side effect and
  succeed, or do nothing and fail.  Inconsistency between side effect
  and claimed success is theoretically possible no matter how success is
  claimed: it's possible if the function returns success/failure, it's
  possible if it sets an Error on failure and doesn't on success, and
  it's possible if it does both.

My point is: returning void instead of success/failure gets rid only of
a part of a theoretical problem, which turns out not much of a problem
in practice.

>                           Especially when the tree is an inconsistent
> state where we mix -1/0, -errno/0, FALSE/TRUE, NULL/non-NULL and void
> functions.

This is basically ret<0/ret>=0, !ret/ret, void.

Getting rid of void would improve matters, wouldn't it?