Re: [qemu-s390x] [RFC] error: auto propagated local_err

[Eric Blake]

On 9/19/19 9:13 AM, Vladimir Sementsov-Ogievskiy wrote:

> 
> With my plan of two different macro, I at least messed the case when we need
> both dereferencing and hints, which means third macro, or one macro with 
> parameters,
> saying what to wrap.
> 
> And my aim was to follow the idea of "do propagation only if it really 
> necessary in this case".
> 
> But may be you are right, and we shouldn't care so much.
> 
> 1. What is bad, if we wrap NULL, when we only want to use hints?
> Seems nothing. Some extra actions on error path, but who cares about it?
> 
> 2. What is bad, if we wrap error_fatal, when we only want to dereference, and 
> don't use hints?
> Seems nothing again, on error path we will return from higher level, and a 
> bit of extra work, but nothing worse..
> 
> So I tend to agree. But honestly, I didn't understand first part of Kevin's 
> paragraph against propagation,
> so, may be he have more reasons to minimize number of cases when we propagate.

Here's the problem.

error_propagate(&error_abort, ...)

produces an abort() with a stack trace tied to your CURRENT location,
not the location at which the error was first detected.  We DO copy the
function name and line number from the original detection, but we have
moved on since that point in time, so the backtrace does not match the
reported location in the error struct.  So for error_abort, you want to
abort as soon as possible, without any intermediate error_propagate - we
use this to flag programmer errors, and want to make life easy for the
programmer.

But for error_propagate(&error_fatal, ...), we WANT to delay things to
as late as possible, in order to gather up as many additional
append_hints as possible, before finally telling the user their problem.
 In that case, we don't care about the stack trace or even the function
and line number of the failure, but about giving the user as much as we
can to help them fix their command-line problem.

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

error_propagate() serves two purposes:

It is a crutch to make it easier to paper over passing NULL as errp (in
that case, we substitute a non-NULL pointer, then we can use 'if
(local_error)', and error_propagate() then frees local_error).  Hiding
this crutch behind a macro is desirable (less boilerplate).

It helps transfer from one error to another.  In this form, we are fine
using it for error_fatal (because we WANT the transfer to happen as late
as possible, because once we transfer the program exits so we can't add
any more hints), but not for error_abort (because we don't want to lose
context by transferring), and wasteful at other times (write directly to
the original error, instead of having to transfer).  So again, hiding it
in a macro means we no longer have to call it directly, but only in the
places where it helps.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org