[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call |
Date: |
Mon, 28 Sep 2015 11:24:30 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 09/24/2015 08:58 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> Due to the existing semantics of the error_set() family,
>>> generated sequences in the qapi visitors such as:
>>>
>>> visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err);
>>> if (!err) {
>>> visit_type_FOO_fields(m, obj, errp);
>>> visit_end_implicit_struct(m, &err);
>>> }
>>> error_propagate(errp, err);
>>
>> Indentation seems off. Intentional?
>>
>>>
>>> are risky: if visit_type_FOO_fields() sets errp, and then
>>> visit_end_implicit_struct() also encounters an error, the
>>> attempt to overwrite the first error will cause an abort().
>
> I didn't even read error_propagate()'s contract correctly. It
> specifically specifies that if errp is already set, then err is ignored.
Yes. Differs from error_set() & friends, where the destination must not
contain an error. The inconsistency is a bit ugly. Perhaps it adds
enough convenience to make it worthwhile. Anyway, changing it now would
be a huge bother.
Note that GLib's g_propagate_error() requires the destination not to
contain an error.
> So the above sequence is actually just fine, because only the following
> paths exist:
>
> visit_start_implicit_struct() fails into &err,
> error_propagate() passes err into caller's errp
>
> visit_start_implicit_struct() succeeds,
> visit_type_FOO_fields() fails into caller's errp,
> visit_end_implicit_struct() succeeds,
> error_propagate() does nothing
>
> visit_start_implicit_struct() succeeds,
> visit_type_FOO_fields() fails into caller's errp,
> visit_end_implicit_struct() fails int &err,
> error_propagate() does nothing (errp trumps err)
Yes, but visit_end_implicit_struct() gets called with an errp argument
that may already contain an error, and that's unusual. Prominent notice
in the contract required.
> visit_start_implicit_struct() succeeds,
> visit_type_FOO_fields() succeeds,
> visit_end_implicit_struct() fails int &err,
> error_propagate() passes err into caller's errp
>
> visit_start_implicit_struct() succeeds,
> visit_type_FOO_fields() succeeds,
> visit_end_implicit_struct() succeeds,
> error_propagate() does nothing
>
>
> As such, I'm revisiting if anything is needed at all, other than making
> the various visit_start/visit_end patterns consistent with one another
> using existing idioms, and it may turn out we don't need the ternary
> after all.
- Re: [Qemu-devel] [PATCH v5 02/46] qapi: Clean up qapi.py per pep8, (continued)
[Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call, Eric Blake, 2015/09/21
Re: [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call, Eric Blake, 2015/09/26
Re: [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call, Eric Blake, 2015/09/26
Re: [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call,
Markus Armbruster <=
[Qemu-devel] [PATCH v5 08/46] qapi: Reuse code for flat union base validation, Eric Blake, 2015/09/21
[Qemu-devel] [PATCH v5 09/46] qapi: Use consistent generated code patterns, Eric Blake, 2015/09/21
[Qemu-devel] [PATCH v5 10/46] qapi: Merge generation of per-member visits, Eric Blake, 2015/09/21
[Qemu-devel] [PATCH v5 11/46] qapi: Don't use info as witness of implicit object type, Eric Blake, 2015/09/21