[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Clean up code generated around
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Clean up code generated around visit_end_union() |
Date: |
Wed, 27 Jan 2016 15:46:28 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 01/27/2016 06:54 AM, Markus Armbruster wrote:
>> The generated code can call visit_end_union() without having called
>> visit_start_union(). Example:
>>
>> if (!*obj) {
>> goto out_obj;
>> }
>> visit_type_BlockdevOptions_fields(v, obj, &err);
>> if (err) {
>> goto out_obj; // if we go from here...
>> }
>> if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) {
>> goto out_obj;
>> }
>> switch ((*obj)->driver) {
>> [...]
>> }
>> out_obj:
>> // ... then *obj is true, and ...
>> error_propagate(errp, err);
>> err = NULL;
>> if (*obj) {
>> // we end up here
>> visit_end_union(v, !!(*obj)->u.data, &err);
>> }
>> error_propagate(errp, err);
Tabs crept into my commit message, oops.
>>
>> Harmless only because no visitor implements end_union(). Clean it up
>> anyway.
>
> I plan on deleting visit_end_union() anyways (and visit_start_union);
> see 32/37, plus the FIXME comments added in 21/37. Maybe it's easier to
> just delete this incorrect and unused callback earlier in the series,
> using your commit message as additional rationale why it is worthless,
> and leaving only visit_start_union() cleanups for 32/37.
I could've tried to move PATCH 32 before the unification, but that
would've been work, so I went with this straightforward fix instead.
Sure, it fixes something that'll go away soon, but I think the churn is
quite tolerable: 1 file changed, 2 insertions(+), 4 deletions(-).
>> Messed up since we have visit_end_union (commit cee2ded).
>
> Indeed.
[Qemu-devel] [PATCH v9 18/37] qapi: Drop unused error argument for list and implicit struct, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 17/37] qapi: Drop unused 'kind' for struct/enum visit, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 29/37] qapi: Eliminate empty visit_type_FOO_fields, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 27/37] qapi: Add type.is_empty() helper, Eric Blake, 2016/01/19