Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> Wrap generated code with #if/#endif using an 'ifcontext' on
> QAPIGenCSnippet objects.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> scripts/qapi/commands.py | 21 ++++++++++++---------
> tests/test-qmp-cmds.c | 5 +++--
> 2 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index dcc03c7859..72749c7fc5 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -239,7 +239,7 @@ class
> QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
> QAPISchemaModularCVisitor.__init__(
> self, prefix, 'qapi-commands',
> ' * Schema-defined QAPI/QMP commands', __doc__)
> - self._regy = ''
> + self._regy = QAPIGenCCode()
> self._visited_ret_types = {}
>
> def _begin_module(self, name):
> @@ -275,20 +275,23 @@ 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, allow_preconfig):
> 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]:
> + if ret_type and \
> + ret_type not in self._visited_ret_types[self._genc]:
PEP8 prefers parenthesis over backslash. Can touch this up when I
apply.
> 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,
> - allow_preconfig)
> + with ifcontext(ret_type.ifcond, self._genh, self._genc,
> self._regy):
> + self._genc.add(gen_marshal_output(ret_type))
> + with ifcontext(ifcond, self._genh, self._genc, self._regy):
> + self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
> + 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, allow_preconfig))
Okay, now it falls apart differently :) Let's step through the code
real slow.
The first time we visit a command C returning type T...
if ret_type and \
ret_type not in self._visited_ret_types[self._genc]:
self._visited_ret_types[self._genc].add(ret_type)
with ifcontext(ret_type.ifcond, self._genh, self._genc, self._regy):
self._genc.add(gen_marshal_output(ret_type))
... we generate qmp_marshal_output_T() wrapped in T's condition.
with ifcontext(ifcond, self._genh, self._genc, self._regy):
self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
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, allow_preconfig))
We generate qmp_marshal_C() wrapped in C's condition. This is what
calls qmp_marshal_output_T().
If T is a user-defined type, the user is responsible for making this
work, i.e. to make T's condition the conjunction of the T-returning
commands' conditions.
If T is a built-in type, this isn't possible: the qmp_marshal_output_T()
will be generated unconditionally.
I append a crude patch to provide test coverage (crude because it
removes coverage of another case). With it applied, make check dies
with
tests/test-qapi-commands.c:470:13: warning: ‘qmp_marshal_output_str’
defined but not used [-Wunused-function]
I think the issue is obscure enough to justify postponing a proper fix,
and just add a FIXME here now. I can do that when I apply.
>
>
> def gen_commands(schema, output_dir, prefix):
> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> index 840530b84c..bd27353908 100644
> --- a/tests/test-qmp-cmds.c
> +++ b/tests/test-qmp-cmds.c
> @@ -12,12 +12,13 @@
>
> static QmpCommandList qmp_commands;
>
> -/* #if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) */
> +#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD)
> UserDefThree *qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
> +void qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
Whoops!
Not caught by make check since it compiles with none of the conditionals
defined. Improvement welcome, but not necessary to get this series in.
I can fix the editing accident when I apply.
> {
> return NULL;
> }
> -/* #endif */
> +#endif
>
> UserDefThree *qmp_TestCmdReturnDefThree(Error **errp)
> {
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 16209b57b3..4dd1ce3703 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -8,7 +8,8 @@
# Commands allowed to return a non-dictionary:
'returns-whitelist': [
'guest-get-time',
- 'guest-sync' ] } }
+ 'guest-sync',
+ 'TestIfCmd' ] } }
{ 'struct': 'TestStruct',
'data': { 'integer': 'int', 'boolean': 'bool', 'string': 'str' } }
@@ -56,9 +57,6 @@
'data': { 'string0': 'str',
'dict1': 'UserDefTwoDict' } }
-{ 'struct': 'UserDefThree',
- 'data': { 'string0': 'str' } }
-
# dummy struct to force generation of array types not otherwise mentioned
{ 'struct': 'ForceArrays',
'data': { 'unused1':['UserDefOne'], 'unused2':['UserDefTwo'],
@@ -212,10 +210,8 @@
'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct' },
- 'returns': 'UserDefThree',
+ 'returns': 'str',
'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
-{ 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' }
-
{ 'event': 'TestIfEvent', 'data': { 'foo': 'TestIfStruct' },
'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 0da92455da..aafa40c226 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -36,8 +36,6 @@ object UserDefTwoDict
object UserDefTwo
member string0: str optional=False
member dict1: UserDefTwoDict optional=False
-object UserDefThree
- member string0: str optional=False
object ForceArrays
member unused1: UserDefOneList optional=False
member unused2: UserDefTwoList optional=False
@@ -257,11 +255,9 @@ alternate TestIfAlternate
object q_obj_TestIfCmd-arg
member foo: TestIfStruct optional=False
if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
-command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
+command TestIfCmd q_obj_TestIfCmd-arg -> str
gen=True success_response=True boxed=False oob=False preconfig=False
if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
-command TestCmdReturnDefThree None -> UserDefThree
- gen=True success_response=True boxed=False oob=False preconfig=False
object q_obj_TestIfEvent-arg
member foo: TestIfStruct optional=False
if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index bd27353908..6ba73fd53c 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -13,18 +13,12 @@
static QmpCommandList qmp_commands;
#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD)
-UserDefThree *qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
-void qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
+char *qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
{
return NULL;
}
#endif
-UserDefThree *qmp_TestCmdReturnDefThree(Error **errp)
-{
- return NULL;
-}
-
void qmp_user_def_cmd(Error **errp)
{
}
