Re: [Qemu-devel] [PATCH v2 1/6] error: Add error_abort

[Markus Armbruster]

Peter Crosthwaite <address@hidden> writes:

> Add a special Error * that can be passed to error handling APIs to
> signal that any errors are fatal and should abort QEMU. There are two
> advantages to this:
>
> - allows for brevity when wishing to assert success of Error **
>   accepting APIs. No need for this pattern:
>         Error * local_err = NULL;
>         api_call(foo, bar, &local_err);
>         assert_no_error(local_err);
>   This also removes the need for _nofail variants of APIs with
>   asserting call sites now reduced to 1LOC.
> - SIGABRT happens from within the offending API. When a fatal error
>   occurs in an API call (when the caller is asserting sucess) failure
>   often means the API itself is broken. With the abort happening in the
>   API call now, the stack frames into the call are available at debug
>   time. In the assert_no_error scheme the abort happens after the fact.
>
> The exact semantic is that when an error is raised, if the argument
> Error ** matches &error_abort, then the abort occurs immediately. The
> error messaged is reported.
>
> For error_propagate, if the destination error is &error_abort, then
> the abort happens at propagation time.
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
> changed since v1:
> Delayed assertions that *errp == NULL.

Care to explain why you want to delay these assertions?  I'm not sure I
get it...

[...]
> @@ -31,7 +33,6 @@ void error_set(Error **errp, ErrorClass err_class, const 
> char *fmt, ...)
>      if (errp == NULL) {
>          return;
>      }
> -    assert(*errp == NULL);
>  
>      err = g_malloc0(sizeof(*err));
>  
> @@ -40,6 +41,12 @@ void error_set(Error **errp, ErrorClass err_class, const 
> char *fmt, ...)
>      va_end(ap);
>      err->err_class = err_class;
>  
> +    if (errp == &error_abort) {
> +        error_report("%s", error_get_pretty(err));
> +        abort();
> +    }
> +
> +    assert(*errp == NULL);
>      *errp = err;
>  }
>  
[...]