Peter Crosthwaite <peter.crosthwa...@xilinx.com> writes:
> On Fri, Dec 6, 2013 at 9:59 PM, Markus Armbruster <arm...@redhat.com> wrote:
>> Eric Blake <ebl...@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.crosthwa...@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...
>
> The assertion evaluates to true so there is no issue leaving it where
> it is. I am perhaps being overly defensive ...
>
>>
>> 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.
>>
>
> In a rather obscure way. Following on from the "what happens if
> someone overwrites error_abort" discussion, I was going for the "limp
> on" approach when someone stores to error abort. My thinking is that
> error abort is potentially corruptable, given it's whole reason to be
> is to trap fatally broken code. Hardening it against a bad pointer
> deref leading up to the fatal error it actually traps may make sense.
> Although I'm probably chasing ghosts there. Happy to revert however
> depending on consensus. Both v1 and v2 are of equivalent functionality
> in the 99.99% case and I have no strong opinion one way or the other.
>
> Votes welcome :)
I'd leave the assertion where it is now.
