From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55100)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1Vou5h-0006Ys-1a
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 07:01:10 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1Vou5c-00048I-0F
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 07:01:04 -0500
Received: from mx1.redhat.com ([209.132.183.28]:64100)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1Vou5b-000483-OM
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 07:00:59 -0500
From: Markus Armbruster <armbru@redhat.com>
References: <cover.1386203851.git.peter.crosthwaite@xilinx.com>
	<46e453b90b9822456f6ffb97f9c03ccbf36a0214.1386203851.git.peter.crosthwaite@xilinx.com>
	<874n6n7ghy.fsf@blackfin.pond.sub.org> <52A07DF0.3010408@redhat.com>
Date: Fri, 06 Dec 2013 12:59:36 +0100
In-Reply-To: <52A07DF0.3010408@redhat.com> (Eric Blake's message of "Thu, 05
	Dec 2013 06:21:52 -0700")
Message-ID: <87pppai40n.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v2 1/6] error: Add error_abort
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: imammedo@redhat.com, Peter Crosthwaite <peter.crosthwaite@xilinx.com>, pbonzini@redhat.com, qemu-devel@nongnu.org, afaerber@suse.de

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.