qapi: Require boxed for conditional command and event arguments

The C code generator fails to honor 'if' conditions of command and
event arguments.

For instance, tests/qapi-schema/qapi-schema-test.json has

    { 'event': 'TEST_IF_EVENT',
      'data': { 'foo': 'TestIfStruct',
		'bar': { 'type': ['str'], 'if': 'TEST_IF_EVT_ARG' } },
      'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }

Generated tests/test-qapi-events.h fails to honor the TEST_IF_EVT_ARG
condition:

    #if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
    void qapi_event_send_test_if_event(TestIfStruct *foo, strList *bar);
    #endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */

Only uses so far are in tests/.

We could fix the generator to emit something like

    #if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
    void qapi_event_send_test_if_event(TestIfStruct *foo
    #if defined(TEST_IF_EVT_ARG)
                    , strList *bar
    #endif
                    );
    #endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */

Ugly.  Calls become similarly ugly.  Not worth fixing.

Conditional arguments work fine with 'boxed': true, simply because
complex types with conditional members work fine.  Not worth breaking.

Reject conditional arguments unless boxed.

Move the tests cases covering unboxed conditional arguments out of
tests/qapi-schema/qapi-schema-test.json.  Cover boxed conditional
arguments there instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20230316071325.492471-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Markus Armbruster 2023-03-16 08:13:25 +01:00
parent 713d921aed
commit de3b3f529d
16 changed files with 48 additions and 22 deletions

View File

@ -805,9 +805,8 @@ gets its generated code guarded like this::
... generated code ...
#endif /* defined(HAVE_BAR) && defined(CONFIG_FOO) */
Individual members of complex types, commands arguments, and
event-specific data can also be made conditional. This requires the
longhand form of MEMBER.
Individual members of complex types can also be made conditional.
This requires the longhand form of MEMBER.
Example: a struct type with unconditional member 'foo' and conditional
member 'bar' ::

View File

@ -66,6 +66,7 @@ def gen_call(name: str,
elif arg_type:
assert not arg_type.variants
for memb in arg_type.members:
assert not memb.ifcond.is_present()
if memb.need_has():
argstr += 'arg.has_%s, ' % c_name(memb.name)
argstr += 'arg.%s, ' % c_name(memb.name)

View File

@ -119,6 +119,7 @@ def build_params(arg_type: Optional[QAPISchemaObjectType],
elif arg_type:
assert not arg_type.variants
for memb in arg_type.members:
assert not memb.ifcond.is_present()
ret += sep
sep = ', '
if memb.need_has():

View File

@ -486,6 +486,10 @@ class QAPISchemaObjectType(QAPISchemaType):
assert self.members is not None
return not self.members and not self.variants
def has_conditional_members(self):
assert self.members is not None
return any(m.ifcond.is_present() for m in self.members)
def c_name(self):
assert self.name != 'q_empty'
return super().c_name()
@ -817,6 +821,11 @@ class QAPISchemaCommand(QAPISchemaEntity):
self.info,
"command's 'data' can take %s only with 'boxed': true"
% self.arg_type.describe())
self.arg_type.check(schema)
if self.arg_type.has_conditional_members() and not self.boxed:
raise QAPISemError(
self.info,
"conditional command arguments require 'boxed': true")
if self._ret_type_name:
self.ret_type = schema.resolve_type(
self._ret_type_name, self.info, "command's 'returns'")
@ -872,6 +881,11 @@ class QAPISchemaEvent(QAPISchemaEntity):
self.info,
"event's 'data' can take %s only with 'boxed': true"
% self.arg_type.describe())
self.arg_type.check(schema)
if self.arg_type.has_conditional_members() and not self.boxed:
raise QAPISemError(
self.info,
"conditional event arguments require 'boxed': true")
def connect_doc(self, doc=None):
super().connect_doc(doc)

View File

@ -0,0 +1,2 @@
args-if-implicit.json: In command 'test-if-cmd':
args-if-implicit.json:1: conditional command arguments require 'boxed': true

View File

@ -0,0 +1,4 @@
{ 'command': 'test-if-cmd',
'data': {
'foo': 'int',
'bar': { 'type': 'str', 'if': 'TEST_IF_CMD_ARG' } } }

View File

View File

@ -0,0 +1,2 @@
args-if-unboxed.json: In command 'test-if-cmd':
args-if-unboxed.json:5: conditional command arguments require 'boxed': true

View File

@ -0,0 +1,6 @@
{ 'struct': 'TestIfCmdArgs',
'data': {
'foo': 'int',
'bar': { 'type': 'str', 'if': 'TEST_IF_CMD_ARG' } } }
{ 'command': 'test-if-cmd',
'data': 'TestIfCmdArgs' }

View File

View File

@ -0,0 +1,2 @@
tests/qapi-schema/event-args-if-unboxed.json: In event 'TEST_IF_EVENT':
tests/qapi-schema/event-args-if-unboxed.json:1: event's 'data' members may have 'if' conditions only with 'boxed': true

View File

@ -0,0 +1,4 @@
{ 'event': 'TEST_IF_EVENT',
'data': {
'foo': 'int',
'bar': { 'type': 'str', 'if': 'TEST_IF_CMD_ARG' } } }

View File

@ -27,6 +27,8 @@ schemas = [
'args-bad-boxed.json',
'args-boxed-anon.json',
'args-boxed-string.json',
'args-if-implicit.json',
'args-if-unboxed.json',
'args-int.json',
'args-invalid.json',
'args-member-array-bad.json',

View File

@ -249,17 +249,16 @@
'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-cmd',
'data': {
'foo': 'TestIfStruct',
'bar': { 'type': 'str', 'if': 'TEST_IF_CMD_ARG' } },
'boxed': true,
'data': 'TestIfStruct',
'returns': 'UserDefThree',
'if': { 'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
{ 'event': 'TEST_IF_EVENT',
'data': { 'foo': 'TestIfStruct',
'bar': { 'type': ['str'], 'if': 'TEST_IF_EVT_ARG' } },
'boxed': true,
'data': 'TestIfStruct',
'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
{ 'event': 'TEST_IF_EVENT2', 'data': {},

View File

@ -283,23 +283,13 @@ object q_obj_test-if-alternate-cmd-arg
command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
if {'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT']}
object q_obj_test-if-cmd-arg
member foo: TestIfStruct optional=False
member bar: str optional=False
if TEST_IF_CMD_ARG
if {'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT']}
command test-if-cmd q_obj_test-if-cmd-arg -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
command test-if-cmd TestIfStruct -> UserDefThree
gen=True success_response=True boxed=True oob=False preconfig=False
if {'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT']}
command test-cmd-return-def-three None -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
object q_obj_TEST_IF_EVENT-arg
member foo: TestIfStruct optional=False
member bar: strList optional=False
if TEST_IF_EVT_ARG
if {'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT']}
event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
boxed=False
event TEST_IF_EVENT TestIfStruct
boxed=True
if {'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT']}
event TEST_IF_EVENT2 None
boxed=False