[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 11/49] qapi/commands: add #if conditions to c
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 11/49] qapi/commands: add #if conditions to commands |
Date: |
Fri, 22 Jun 2018 10:34:18 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> Wrap generated code with #if/#endif using an 'ifcontext' on
> QAPIGenCSnippet objects.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> scripts/qapi/commands.py | 19 ++++++++++---------
> tests/test-qmp-cmds.c | 4 ++--
> 2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index e2366b4801..40bb680b7c 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -237,7 +237,7 @@ class
> QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
> QAPISchemaModularCVisitor.__init__(
> self, prefix, 'qapi-commands',
> ' * Schema-defined QAPI/QMP commands', __doc__)
> - self._regy = ''
> + self._regy = QAPIGenCSnippet()
> self._visited_ret_types = {}
>
> def _begin_module(self, name):
> @@ -273,19 +273,20 @@ class
> QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
> void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
> ''',
> c_prefix=c_name(self._prefix, protect=False)))
> - genc.add(gen_registry(self._regy, self._prefix))
> + genc.add(gen_registry(self._regy.get_content(), self._prefix))
>
> def visit_command(self, name, info, ifcond, arg_type, ret_type,
> gen, success_response, boxed, allow_oob):
> if not gen:
> return
> - self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
> - if ret_type and ret_type not in self._visited_ret_types[self._genc]:
> - self._visited_ret_types[self._genc].add(ret_type)
> - self._genc.add(gen_marshal_output(ret_type))
> - self._genh.add(gen_marshal_decl(name))
> - self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
> - self._regy += gen_register_command(name, success_response, allow_oob)
> + with ifcontext(ifcond, self._genh, self._genc, self._regy):
> + self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
> + if ret_type and ret_type not in
> self._visited_ret_types[self._genc]:
> + self._visited_ret_types[self._genc].add(ret_type)
> + self._genc.add(gen_marshal_output(ret_type))
I'm afraid this falls apart when multiple commands with different
conditions share a return type.
That case needs test coverage.
Aside: the qmp_marshal_FOO() should be static. I can see just two uses
preventing that:
monitor.c:1146: qmp_marshal_qmp_capabilities,
QCO_NO_OPTIONS);
monitor.c:4312: qmp_marshal_query_version(NULL, &ver, NULL);
Would be nice to get rid of those. Not necessarily in this series, of
course.
> + self._genh.add(gen_marshal_decl(name))
> + self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
> + self._regy.add(gen_register_command(name, success_response,
> allow_oob))
>
>
> def gen_commands(schema, output_dir, prefix):
> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> index c25fc2100a..e675722593 100644
> --- a/tests/test-qmp-cmds.c
> +++ b/tests/test-qmp-cmds.c
> @@ -12,11 +12,11 @@
>
> static QmpCommandList qmp_commands;
>
> -/* #if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) */
> +#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD)
> void qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
> {
> }
> -/* #endif */
> +#endif
>
> void qmp_user_def_cmd(Error **errp)
> {