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

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 12/05/2013 03:13 AM, Markus Armbruster wrote:
>
>>>
>>> For error_propagate, if the destination error is &error_abort, then
>>> the abort happens at propagation time.
>>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>> 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...
>
> error_abort as a global variable is always NULL.
>
>> 
>> [...]
>>> @@ -31,7 +33,6 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>>>      if (errp == NULL) {
>>>          return;
>>>      }
>>> -    assert(*errp == NULL);
>
> So *&error_abort is null and this assertion would fire, unless we delay
> the check for NULL...

Err, one of us is confused :)

When errp == &error_abort, then *errp should be null.  If it isn't, then
something got stored in error_abort, which is quite wrong.  Leaving the
assertion where it is catches that.

>>>  
>>>      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);
>
> ...until after the check for &error_abort.