OpenE2K
/
qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

qapi script: do not allow string discriminator 

Since enum based discriminators provide better type-safety and
ensure that future qapi additions do not forget to adjust dependent
unions, forbid using string as discriminator from now on.

Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This commit is contained in:
Wenchao Xia 2014-03-04 18:44:39 -08:00 committed by Luiz Capitulino
parent 59ca664ef8
commit 5223070c47
12 changed files with 53 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
docs/qapi-code-gen.txt
View File

@ -123,10 +123,7 @@ And it looks like this on the wire:
Flat union types avoid the nesting on the wire. They are used whenever a
specific field of the base type is declared as the discriminator ('type' is
then no longer generated). The discriminator can be a string field or a
predefined enum field. If it is a string field, a hidden enum type will be
generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
will be done to verify the correctness. It is recommended to use an enum field.
then no longer generated). The discriminator must be of enumeration type.
The above example can then be modified as follows:
{ 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }

5
scripts/qapi.py
View File

@ -239,6 +239,11 @@ def check_union(expr, expr_info):
"type '%s'"
% (discriminator, base))
enum_define = find_enum(discriminator_type)
# Do not allow string discriminator
if not enum_define:
raise QAPIExprError(expr_info,
"Discriminator '%s' must be of enumeration "
"type" % discriminator)
# Check every branch
for (key, value) in members.items():

3
tests/Makefile
View File

@ -145,7 +145,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
unclosed-list.json unclosed-object.json unclosed-string.json \
duplicate-key.json union-invalid-base.json flat-union-no-base.json \
flat-union-invalid-discriminator.json \
flat-union-invalid-branch-key.json flat-union-reverse-define.json)
flat-union-invalid-branch-key.json flat-union-reverse-define.json \
flat-union-string-discriminator.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h

1
tests/qapi-schema/flat-union-string-discriminator.err Normal file
View File

@ -0,0 +1 @@
<stdin>:13: Discriminator 'kind' must be of enumeration type

1
tests/qapi-schema/flat-union-string-discriminator.exit Normal file
View File

@ -0,0 +1 @@
1

17
tests/qapi-schema/flat-union-string-discriminator.json Normal file
View File

@ -0,0 +1,17 @@
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'type': 'TestBase',
'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
{ 'type': 'TestTypeA',
'data': { 'string': 'str' } }
{ 'type': 'TestTypeB',
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
'base': 'TestBase',
'discriminator': 'kind',
'data': { 'kind1': 'TestTypeA',
'kind2': 'TestTypeB' } }

0
tests/qapi-schema/flat-union-string-discriminator.out Normal file
View File

9
tests/qapi-schema/qapi-schema-test.json
View File

@ -37,10 +37,13 @@
'base': 'UserDefZero',
'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
{ 'type': 'UserDefUnionBase',
'data': { 'string': 'str', 'enum1': 'EnumOne' } }
{ 'union': 'UserDefFlatUnion',
'base': 'UserDefOne',
'discriminator': 'string',
'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
'base': 'UserDefUnionBase',
'discriminator': 'enum1',
'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } }
# FIXME generated struct UserDefFlatUnion has members for direct base
# UserDefOne, but lacks members for indirect base UserDefZero

5
tests/qapi-schema/qapi-schema-test.out
View File

@ -7,7 +7,8 @@
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefOne'), ('discriminator', 'string'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
OrderedDict([('union', 'UserDefAnonUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]),
OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]),
OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
@ -17,7 +18,6 @@
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': 'UserDefUnionKind', 'enum_values': None},
{'enum_name': 'UserDefFlatUnionKind', 'enum_values': None},
{'enum_name': 'UserDefAnonUnionKind', 'enum_values': None},
{'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
@ -27,4 +27,5 @@
OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]

5
tests/test-qmp-input-strict.c
View File

@ -146,7 +146,10 @@ static void test_validate_union_flat(TestInputVisitorData *data,
Visitor *v;
Error *errp = NULL;
v = validate_test_init(data, "{ 'string': 'a', 'boolean': true }");
v = validate_test_init(data,
"{ 'enum1': 'value1', "
"'string': 'str', "
"'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp);

10
tests/test-qmp-input-visitor.c
View File

@ -310,14 +310,18 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
Error *err = NULL;
UserDefFlatUnion *tmp;
v = visitor_input_test_init(data, "{ 'string': 'a', 'boolean': true }");
v = visitor_input_test_init(data,
"{ 'enum1': 'value1', "
"'string': 'str', "
"'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
g_assert(err == NULL);
g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_A);
g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1);
g_assert_cmpstr(tmp->string, ==, "str");
/* TODO g_assert_cmpint(tmp->integer, ==, 41); */
g_assert_cmpint(tmp->a->boolean, ==, true);
g_assert_cmpint(tmp->value1->boolean, ==, true);
qapi_free_UserDefFlatUnion(tmp);
}

10
tests/test-qmp-output-visitor.c
View File

@ -449,10 +449,11 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
Error *err = NULL;
UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
tmp->kind = USER_DEF_UNION_KIND_A;
tmp->a = g_malloc0(sizeof(UserDefA));
tmp->kind = ENUM_ONE_VALUE1;
tmp->string = g_strdup("str");
tmp->value1 = g_malloc0(sizeof(UserDefA));
/* TODO when generator bug is fixed: tmp->integer = 41; */
tmp->a->boolean = true;
tmp->value1->boolean = true;
visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err);
g_assert(err == NULL);
@ -461,7 +462,8 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
g_assert(qobject_type(arg) == QTYPE_QDICT);
qdict = qobject_to_qdict(arg);
g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "a");
g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value1");
g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "str");
/* TODO g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); */
g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true);