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.
> 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.
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
