Re: [RFC] error: auto propagated local_err

[Daniel P . Berrangé]

On Thu, Sep 19, 2019 at 09:44:14AM -0500, Eric Blake wrote:
> On 9/19/19 9:30 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
> >>
> >> To the same topic, of minimization: should we always call MAKE_ERRP_SAFE 
> >> at function top, or only
> >> in block, where it is needed (assume, we dereference it only inside some 
> >> "if" or "while"?
> >> Kevin, is something bad in propagation, when it not related to error_abort?
> >>
> >>
> > 
> > Or, even, we may use MAKE_ERRP_SAFE on every function, which have Error 
> > **errp argument, even if we neither
> > dereference it nor append hints? Is it what you propose by "SINGLE 
> > paradigm"? It's of course simpler to script,
> > to check in checkpatch and to maintain.
> 
> Yes. The simpler we make the rules, and the less boilerplate it entails,
> the more likely that we can:
> 
> a) avoid exceptions and corner cases that cost review time
> b) automate the conversion into complying with the rule
> c) codify those rules in syntax check to ensure they are followed
> post-conversion
> 
> ALWAYS using MAKE_ERRP_SAFE() on entry to any function that has an Error
> **errp parameter is dirt-simple to explain.  It has no performance
> penalty if the user passed in a normal error or error_abort (the cost of
> an 'if' hidden in the macro is probably negligible compared to
> everything else we do), and has no semantic penalty if the user passed
> in NULL or error_fatal (we now get the behavior we want with less
> boilerplate).
> 
> Having to think 'does this method require me to use MAKE_ERRP_SAFE, or
> can I omit it?' does not provide the same simplicity.

The flipside is that MAKE_ERRP_SAFE hides a bunch of logic, so you don't
really know what its doing without looking at it, and this is QEMU
custom concept so one more thing to learn for new contributors.

While I think it is a nice trick, personally I think we would be better
off if we simply used a code pattern which does not require de-referencing
'errp' at all, aside from exceptional cases. IOW, no added macro in 95%
of all our methods using Error **errp.

There are lots of neat things we could do with auto-cleanup functions we
I think we need to be wary of hiding too much cleverness behind macros
when doing so overall.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|