Eric Blake <ebl...@redhat.com> writes:
> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>> Fixes events whose data is struct with base to include the struct's
>> base members. Test case is qapi-schema-test.json's event
>> __org.qemu_x-command:
>>
>> { 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }
>>
>> { 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
>> 'data': { '__org.qemu_x-member2': 'str' } }
>>
>> { 'struct': '__org.qemu_x-Base',
>> 'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
>
> No change to generated code in qemu proper, so this is a corner case we
> are not yet exploiting. But good to have it fixed :)
>
>>
>> Patch's effect on generated qapi_event_send___org_qemu_x_event():
>>
>> -void qapi_event_send___org_qemu_x_event(const char
>> *__org_qemu_x_member2,
>> +void qapi_event_send___org_qemu_x_event(__org_qemu_x_Enum
>> __org_qemu_x_member1,
>> + const char
>> *__org_qemu_x_member2,
>> Error **errp)
>
> Ouch - I think we have a bug in qapi.py:check_event(). There, we call
> check_type(... allow_metas=['union', 'struct']) - but it looks like the
> generated signature requires that we have no variants, which means we
> cannot have:
>
> { 'union': 'Un', 'data': ... }
> { 'event': 'EV', 'data': 'Un' }
>
> because it would fail C generation. Sounds like you should add a check
> to that in 14/47, and a fix for it in 15/47.
Will do.
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> scripts/qapi-event.py | 87 ++++++++++++++++-----------------
>> tests/qapi-schema/qapi-schema-test.json | 3 --
>> 2 files changed, 43 insertions(+), 47 deletions(-)
>>
>> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
>> index 537da17..456e590 100644
>> --- a/scripts/qapi-event.py
>> +++ b/scripts/qapi-event.py
>> @@ -2,28 +2,29 @@
>> # QAPI event generator
>> #
>> # Copyright (c) 2014 Wenchao Xia
>> +# Copyright (c) 2013-2015 Red Hat Inc.
>
> Umm - the file didn't exist until 2014; and to my knowledge, you aren't
> adding anything to it that was copied from some other file dating back
> to 2013. Using the range 2014-2015 might be better.
Pasto, will fix.
> But that's minor. And assuming that you reject union types for events
> in an earlier patch, the rest of this is fine:
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
>> @@ -89,15 +90,15 @@ def generate_event_implement(api_name, event_name,
>> params):
>> """,
>> event_name = event_name)
>>
>> - for argname, argentry, optional in parse_args(params):
>> - if optional:
>> + for memb in params.members:
>> + if memb.optional:
>> ret += mcgen("""
>> if (has_%(var)s) {
>> """,
>> - var = c_name(argname))
>> + var=c_name(memb.name))
>> push_indent()
>>
>> - if argentry == "str":
>> + if memb.type.name == "str":
>> var_type = "(char **)"
>
> Our visitors require us to cast away const. Is that something we should
> consider reworking, so that we don't need to do that? But it's a
> question for another day.
Yes, it's annoying, and yes, we need to leave it for later.
Thanks!
