Re: [PATCH v3 04/25] error: auto propagated local_err

[Kevin Wolf]

Am 30.09.2019 um 18:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 30.09.2019 19:00, Kevin Wolf wrote:
> > Am 30.09.2019 um 17:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >> 30.09.2019 18:12, Kevin Wolf wrote:
> >>> Am 24.09.2019 um 22:08 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >>>> Here is introduced ERRP_FUNCTION_BEGIN macro, to be used at start of
> >>>> functions with errp parameter.
> >>>
> >>> A bit of bike shedding, but FOO_BEGIN suggests to me that a FOO_END will
> >>> follow. Can we find a different name, especially now that we won't use
> >>> this macro in every function that uses an errp, so even the "errp
> >>> function" part isn't really correct any more?
> >>>
> >>> How about ERRP_AUTO_PROPAGATE?
> >>
> >> I have an idea that with this macro we can (optionally) get the whole call 
> >> stack
> >> of the error and print it to log, so it's good to give it more generic 
> >> name, not
> >> limited to propagation..
> > 
> > Hm, what's the context for this feature?
> > 
> > The obvious one where you want to have a stack trace is &error_abort,
> > but that one crashes, so you get it automatically. If it's just a normal
> > error (like a QAPI option contains an invalid value and some function
> > down the call chain checks it), why would anyone want to know what the
> > call chain in the QEMU code was?
> > 
> 
> When I have bug from testers, call stack would be a lot more descriptive, 
> than just
> an error message.
> 
> We may add trace point which will print this information, so with disabled 
> trace point
> - no extra output.

But wouldn't it make much more sense then to optionally add this
functionality to any trace point? I really don't see how this is related
specifically to user-visible error messages.

However, even if we decide that we want to have this in Error objects,
wouldn't it make much more sense to use the real C stack trace and save
it from the innermost error_set() using backtrace() or compiler
built-ins rather than relying on an error_propagate() chain?

Kevin