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

[Eduardo Habkost]

On Mon, Jul 03, 2017 at 03:21:56PM +0200, Markus Armbruster wrote:
> 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.
> 

You are probably right.  And I guess we will find out quickly if this is
not the case and the conversion to bool starts introducing more complex
or buggy code.


> >                           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?

Yes.

-- 
Eduardo