[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 12/15] qapi: Don't box struct branch of alte
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v11 12/15] qapi: Don't box struct branch of alternate |
Date: |
Thu, 18 Feb 2016 19:56:58 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 02/18/2016 09:43 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> There's no reason to do two malloc's for an alternate type visiting
>>> a QAPI struct; let's just inline the struct directly as the C union
>>> branch of the struct.
>>>
>>>
>>> meanwhile, in qapi-visit.h, we trade the previous visit_type_FOO(obj)
>>
>> Meanwhile
>>
>>> (which allocates a pointer, handles a layer of {} from the JSON stream,
>>> and visits the fields)
>
> Maybe:
>
> (which first calls visit_start_struct(v, name, obj, size, &err) to
> allocate a pointer and handle a layer of {} from the JSON stream, then
> visits the fields)
>
>>> with an inline call to visit_type_FOO(NULL) (to
>>> consume the {} without allocation) and a direct visit of the fields:
>>
>> I don't see a call to visit_type_FOO(NULL).
>
> Whoops, wrong function name. I meant:
>
> visit_start_struct(v, name, NULL, 0, &err).
> ^^^^
> Especially useful if you take my earlier text suggestion to point out
> the difference in the visit_start_struct() calls as being the
> intentional so that we continue to consume {} but now opt out of a
> second allocation.
Better, but the sentence is long enough to confuse even a German. What
about:
The corresponding spot in qapi-visit.c calls visit_type_FOO(), which
first calls visit_start_struct() to allocate or deallocate the member
and handle a layer of {} from the JSON stream, then visits the
members. To peel off the indirection and the memory management that
comes with it, we inline this call, then suppress allocation /
deallocation by passing NULL to visit_start_struct(), and adjust the
member visit:
>>
>> Suggest not to abbreviate argument lists, it's mildly confusing.
>
> Sure. Are we still at a point that you could touch up the commit message
> without a patch respin?
Looks like it so far.
>>
>> Patch looks good.
>>
[Qemu-devel] [PATCH v11 13/15] qapi: Don't box branches of flat unions, Eric Blake, 2016/02/18
Re: [Qemu-devel] [PATCH v11 00/15] prune some QAPI visitor cruft (was qapi cleanups subset E), Markus Armbruster, 2016/02/18
Re: [Qemu-devel] [PATCH v11 00/15] prune some QAPI visitor cruft (was qapi cleanups subset E), Markus Armbruster, 2016/02/18