From 10d4d997f86cf2a4ce89145df5658952d5722e56 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 4 May 2015 09:05:23 -0600 Subject: qapi: Whitelist commands that don't return dictionary ...or an array of dictionaries. Although we have to cater to existing commands, returning a non-dictionary means the command is not extensible (no new name/value pairs can be added if more information must be returned in parallel). By making the whitelist explicit, any new command that falls foul of this practice will have to be self-documenting, which will encourage developers to either justify the action or rework the design to use a dictionary after all. It's a little bit sloppy that we share a single whitelist among three clients (it's too permissive for each). If this is a problem, a future patch could tighten things by having the generator take the whitelist as an argument (as in scripts/qapi-commands.py --legacy-returns=...), or by having the generator output C code that requires explicit use of the whitelist (as in: #ifndef FROBNICATE_LEGACY_RETURN_OK # error Command 'frobnicate' should return a dictionary #endif then having the callers define appropriate macros). But until we need such fine-grained separation (if ever), this patch does the job just fine. Signed-off-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- tests/qapi-schema/returns-alternate.err | 1 + tests/qapi-schema/returns-alternate.exit | 2 +- tests/qapi-schema/returns-alternate.json | 2 +- tests/qapi-schema/returns-alternate.out | 4 ---- tests/qapi-schema/returns-int.json | 3 ++- tests/qapi-schema/returns-int.out | 2 +- tests/qapi-schema/returns-whitelist.err | 1 + tests/qapi-schema/returns-whitelist.exit | 2 +- tests/qapi-schema/returns-whitelist.json | 2 +- tests/qapi-schema/returns-whitelist.out | 7 ------- 10 files changed, 9 insertions(+), 17 deletions(-) (limited to 'tests/qapi-schema') diff --git a/tests/qapi-schema/returns-alternate.err b/tests/qapi-schema/returns-alternate.err index e69de29bb2..dfbb419cac 100644 --- a/tests/qapi-schema/returns-alternate.err +++ b/tests/qapi-schema/returns-alternate.err @@ -0,0 +1 @@ +tests/qapi-schema/returns-alternate.json:3: 'returns' for command 'oops' cannot use alternate type 'Alt' diff --git a/tests/qapi-schema/returns-alternate.exit b/tests/qapi-schema/returns-alternate.exit index 573541ac97..d00491fd7e 100644 --- a/tests/qapi-schema/returns-alternate.exit +++ b/tests/qapi-schema/returns-alternate.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/returns-alternate.json b/tests/qapi-schema/returns-alternate.json index b3b91fd3d4..972390c06b 100644 --- a/tests/qapi-schema/returns-alternate.json +++ b/tests/qapi-schema/returns-alternate.json @@ -1,3 +1,3 @@ -# FIXME: we should reject returns if it is an alternate type +# we reject returns if it is an alternate type { 'alternate': 'Alt', 'data': { 'a': 'int', 'b': 'str' } } { 'command': 'oops', 'returns': 'Alt' } diff --git a/tests/qapi-schema/returns-alternate.out b/tests/qapi-schema/returns-alternate.out index 8a03ed3adf..e69de29bb2 100644 --- a/tests/qapi-schema/returns-alternate.out +++ b/tests/qapi-schema/returns-alternate.out @@ -1,4 +0,0 @@ -[OrderedDict([('alternate', 'Alt'), ('data', OrderedDict([('a', 'int'), ('b', 'str')]))]), - OrderedDict([('command', 'oops'), ('returns', 'Alt')])] -[{'enum_name': 'AltKind', 'enum_values': None}] -[] diff --git a/tests/qapi-schema/returns-int.json b/tests/qapi-schema/returns-int.json index 7888fb1b08..870ec6366b 100644 --- a/tests/qapi-schema/returns-int.json +++ b/tests/qapi-schema/returns-int.json @@ -1,2 +1,3 @@ # It is okay (although not extensible) to return a non-dictionary -{ 'command': 'okay', 'returns': 'int' } +# But to make it work, the name must be in a whitelist +{ 'command': 'guest-get-time', 'returns': 'int' } diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out index 36b00a9f3b..70b3ac5e6f 100644 --- a/tests/qapi-schema/returns-int.out +++ b/tests/qapi-schema/returns-int.out @@ -1,3 +1,3 @@ -[OrderedDict([('command', 'okay'), ('returns', 'int')])] +[OrderedDict([('command', 'guest-get-time'), ('returns', 'int')])] [] [] diff --git a/tests/qapi-schema/returns-whitelist.err b/tests/qapi-schema/returns-whitelist.err index e69de29bb2..a41f019a52 100644 --- a/tests/qapi-schema/returns-whitelist.err +++ b/tests/qapi-schema/returns-whitelist.err @@ -0,0 +1 @@ +tests/qapi-schema/returns-whitelist.json:10: 'returns' for command 'no-way-this-will-get-whitelisted' cannot use built-in type 'array of int' diff --git a/tests/qapi-schema/returns-whitelist.exit b/tests/qapi-schema/returns-whitelist.exit index 573541ac97..d00491fd7e 100644 --- a/tests/qapi-schema/returns-whitelist.exit +++ b/tests/qapi-schema/returns-whitelist.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/returns-whitelist.json b/tests/qapi-schema/returns-whitelist.json index 8328563b4e..e8b3cea396 100644 --- a/tests/qapi-schema/returns-whitelist.json +++ b/tests/qapi-schema/returns-whitelist.json @@ -1,4 +1,4 @@ -# FIXME: we should enforce that 'returns' be a dict or array of dict unless whitelisted +# we enforce that 'returns' be a dict or array of dict unless whitelisted { 'command': 'human-monitor-command', 'data': {'command-line': 'str', '*cpu-index': 'int'}, 'returns': 'str' } diff --git a/tests/qapi-schema/returns-whitelist.out b/tests/qapi-schema/returns-whitelist.out index 2adcd8ba10..e69de29bb2 100644 --- a/tests/qapi-schema/returns-whitelist.out +++ b/tests/qapi-schema/returns-whitelist.out @@ -1,7 +0,0 @@ -[OrderedDict([('command', 'human-monitor-command'), ('data', OrderedDict([('command-line', 'str'), ('*cpu-index', 'int')])), ('returns', 'str')]), - OrderedDict([('enum', 'TpmModel'), ('data', ['tpm-tis'])]), - OrderedDict([('command', 'query-tpm-models'), ('returns', ['TpmModel'])]), - OrderedDict([('command', 'guest-get-time'), ('returns', 'int')]), - OrderedDict([('command', 'no-way-this-will-get-whitelisted'), ('returns', ['int'])])] -[{'enum_name': 'TpmModel', 'enum_values': ['tpm-tis']}] -[] -- cgit v1.2.1