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