[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 07/35] qapi: Improve generated event use of q
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v8 07/35] qapi: Improve generated event use of qapi visitor |
Date: |
Tue, 5 Jan 2016 08:21:38 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 |
On 01/05/2016 07:07 AM, Marc-André Lureau wrote:
> Hi
>
> On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake <address@hidden> wrote:
>> All other successful clients of visit_start_struct() were paired
>> with an unconditional visit_end_struct(); but the generated
>> code for events was relying on qmp_output_visitor_cleanup() to
>> work on an incomplete visit. Alter the code to guarantee that
>> the struct is completed, which will make a future patch to
>> split visit_end_struct() easier to reason about. While at it,
>> drop some assertions and comments that are not present in other
>> uses of the qmp output visitor, rearrange the declaration to
>> make it easier for a future patch to introduce the notion of
>> a boxed event visit, and pass NULL rather than "" as the 'kind'
>> parameter (matching most other uses where obj is NULL).
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> ---
>> v8: no change
>> v7: place earlier in series, adjust handling of 'kind'
>> v6: new patch
>>
>> If desired, I can defer the hunk re-ordering the declaration of
>> obj to later in the series where it actually comes in handy.
See [1]
>> ---
>> scripts/qapi-event.py | 14 ++++++--------
>> scripts/qapi.py | 5 +++--
>> 2 files changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
>> index 720486f..e37c07a 100644
>> --- a/scripts/qapi-event.py
>> +++ b/scripts/qapi-event.py
>> @@ -41,9 +41,9 @@ def gen_event_send(name, arg_type):
>>
>> if arg_type and arg_type.members:
>> ret += mcgen('''
>> + QObject *obj;
>> QmpOutputVisitor *qov;
>> Visitor *v;
>> - QObject *obj;
>
> This looks like churning code.
This is my comment [1], and I am fine if we sink it to occur as part of
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html
>>
>> - /* Fake visit, as if all members are under a structure */
>> - visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
>> + visit_start_struct(v, NULL, NULL, "%(name)s", 0, &err);
>
> The comment seemed somewhat useful to me.
The pattern visit_start_struct(v, NULL, NULL, name, 0, &err) occurs in
other places in the code without the comment, so it felt like a common
enough idiom that it didn't need special-casing here. Also, even if we
want a comment, I didn't see a reason for the comment to be present in
the generated code multiple times (generated once per event); at most, a
simple Python '# comment' prior to the mcgen() would probably be
sufficient if we still think the comment adds anything.
>
> Reviewed-by: Marc-André Lureau <address@hidden>
>
I guess when Markus gets back he can weigh in on it.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature