On 06/16/2015 06:53 AM, Michael S. Tsirkin wrote:
> It's a common idiom:
>
> Error *local_err = NULL;
> ....
> foo(&local_err);
> ...
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
>
> Unfortunately it means that call to foo(&local_err) will
> not abort even if errp is set to error_abort.
>
> Instead, we get an abort at error_propagate which is too late.That is, the quality of the stack trace is degraded in that it no longer pinpoints the actual cause of failure. > > To fix, add an API to check errp and set local_err to error_abort > if errp is error_abort. > > Signed-off-by: Michael S. Tsirkin <[email protected]> > --- > include/qapi/error.h | 5 +++++ > util/error.c | 5 +++++ > 2 files changed, 10 insertions(+) I like the idea. > +++ b/util/error.c > @@ -28,6 +28,11 @@ static bool error_is_abort(Error **errp) > return errp && *errp && (*errp)->err_class == ERROR_CLASS_MAX; > } > > +Error *error_init_local(Error **errp) > +{ > + return error_is_abort(errp) ? *errp : NULL; What you have works, but see also my ideas in my (latest) reply to 1/3 where you could use pointer equality on error_abort (rather than on &error_abort) as the key factor. After all, the way you have implemented things so far, when taking the *errp branch, you are still effectively returning the error_abort global pointer (you haven't yet used anything that required struct copying of the contents of error_abort). Reviewed-by: Eric Blake <[email protected]> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
