QAPI patches for 2017-05-31

-----BEGIN PGP SIGNATURE-----
 
 iQIcBAABAgAGBQJZLvgfAAoJEDhwtADrkYZTX4IQAJ+iGb4Z9kQE1mbsp68HUf9y
 SuUYx/5yrd0+nqPRiElvd3o21I+T37RlrrDVzQXV03U7vSiX1k4CdEWcnjCvSAMb
 A5kemTGL25MdFq29sqy30e7d3NwQU4Zn4Wd4Ur2VvfJNGlPCMk2jOr1/aTRBd+cj
 SOQw7lFP3Jelw7YpPvJBYv8W2UQaoSMvl4g1gLeyP8ohlLXjf9R/RuUTLqRX9FB5
 lU5V/XEKffrMeXb+rDjfguA6U7B37QjMgzhv1bUCQ8XjWieQdzaTw99cJC2W9UPV
 1R6b0L8Teh5HQWg+Ah6LdI30AaEW6Phb6bo/VdmBYnJsOOCYIId/PjiZPDCIOR3+
 anYvpCPgcBqtyjDF1ATyKDQnZlhnHMtsQQHfcCNJS9zP1R++JWXluKdcvAKGqfTM
 i4g98zZL7WGUgi6pV2VwCsPD6FxUarTdeG4e3VnBcmnMgoLfr6Sz2N9+FYdkUAhQ
 0jMo/vfX6R/covwZIsgm6kCATOURbLudC4mZh3be+KORn/ZI95i7TBJdXu0WeHr7
 FsPWRSz7HKAk7OE1nIAuPwoD5LJBAnK1mX7SZNsE1SXJoSl73xzz0M591Tv53aPM
 aw3zVsh95I2gs950RIlQBlkD92QZmhS1A6frE87TEEdbPIkZLnEMLsRJYaCkyPJe
 W0Mv3KX16cwdVTaJEhB7
 =nbAS
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-05-31' into staging

QAPI patches for 2017-05-31

# gpg: Signature made Wed 31 May 2017 18:06:39 BST
# gpg:                using RSA key 0x3870B400EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* remotes/armbru/tags/pull-qapi-2017-05-31:
  qapi: Reject alternates that can't work with keyval_parse()
  tests/qapi-schema: Avoid 'str' in alternate test cases
  qapi: Document visit_type_any() issues with keyval input
  qobject-input-visitor: Reject non-finite numbers with keyval

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2017-06-01 16:39:16 +01:00
commit 43771d5d92
27 changed files with 140 additions and 88 deletions

View File

@ -607,6 +607,10 @@ void visit_type_number(Visitor *v, const char *name, double *obj,
* @obj must be non-NULL. Input visitors set *@obj to the value;
* other visitors will leave *@obj unchanged. *@obj must be non-NULL
* for output visitors.
*
* Note that some kinds of input can't express arbitrary QObject.
* E.g. the visitor returned by qobject_input_visitor_new_keyval()
* can't create numbers or booleans, only strings.
*/
void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp);

View File

@ -13,6 +13,7 @@
*/
#include "qemu/osdep.h"
#include <math.h>
#include "qapi/error.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/visitor-impl.h"
@ -568,7 +569,7 @@ static void qobject_input_type_number_keyval(Visitor *v, const char *name,
errno = 0;
*obj = strtod(str, &endp);
if (errno || endp == str || *endp) {
if (errno || endp == str || *endp || !isfinite(*obj)) {
/* TODO report -ERANGE more nicely */
error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
full_name(qiv, name), "number");

View File

@ -812,11 +812,26 @@ def check_alternate(expr, info):
if not qtype:
raise QAPISemError(info, "Alternate '%s' member '%s' cannot use "
"type '%s'" % (name, key, value))
if qtype in types_seen:
conflicting = set([qtype])
if qtype == 'QTYPE_QSTRING':
enum_expr = enum_types.get(value)
if enum_expr:
for v in enum_expr['data']:
if v in ['on', 'off']:
conflicting.add('QTYPE_QBOOL')
if re.match(r'[-+0-9.]', v): # lazy, could be tightened
conflicting.add('QTYPE_QINT')
conflicting.add('QTYPE_QFLOAT')
else:
conflicting.add('QTYPE_QINT')
conflicting.add('QTYPE_QFLOAT')
conflicting.add('QTYPE_QBOOL')
if conflicting & set(types_seen):
raise QAPISemError(info, "Alternate '%s' member '%s' can't "
"be distinguished from member '%s'"
% (name, key, types_seen[qtype]))
types_seen[qtype] = key
for qt in conflicting:
types_seen[qt] = key
def check_enum(expr, info):

View File

@ -342,6 +342,8 @@ qapi-schema += alternate-array.json
qapi-schema += alternate-base.json
qapi-schema += alternate-clash.json
qapi-schema += alternate-conflict-dict.json
qapi-schema += alternate-conflict-enum-bool.json
qapi-schema += alternate-conflict-enum-int.json
qapi-schema += alternate-conflict-string.json
qapi-schema += alternate-empty.json
qapi-schema += alternate-nested.json

View File

@ -5,4 +5,4 @@
# the implicit Alt1Kind enum, we would still have a collision with the
# resulting C union trying to have two members named 'a_b'.
{ 'alternate': 'Alt1',
'data': { 'a-b': 'str', 'a_b': 'int' } }
'data': { 'a-b': 'bool', 'a_b': 'int' } }

View File

@ -1,4 +1,4 @@
# we reject alternates with multiple object branches
# alternate branches of object type conflict with each other
{ 'struct': 'One',
'data': { 'name': 'str' } }
{ 'struct': 'Two',

View File

@ -0,0 +1 @@
tests/qapi-schema/alternate-conflict-enum-bool.json:4: Alternate 'Alt' member 'two' can't be distinguished from member 'one'

View File

@ -0,0 +1 @@
1

View File

@ -0,0 +1,6 @@
# alternate branch of 'enum' type that conflicts with bool
{ 'enum': 'Enum',
'data': [ 'aus', 'off' ] }
{ 'alternate': 'Alt',
'data': { 'one': 'Enum',
'two': 'bool' } }

View File

@ -0,0 +1 @@
tests/qapi-schema/alternate-conflict-enum-int.json:4: Alternate 'Alt' member 'two' can't be distinguished from member 'one'

View File

@ -0,0 +1 @@
1

View File

@ -0,0 +1,6 @@
# alternate branches of 'enum' type that conflicts with numbers
{ 'enum': 'Enum',
'data': [ '1', '2', '3' ] }
{ 'alternate': 'Alt',
'data': { 'one': 'Enum',
'two': 'int' } }

View File

@ -1 +1 @@
tests/qapi-schema/alternate-conflict-string.json:4: Alternate 'Alt' member 'two' can't be distinguished from member 'one'
tests/qapi-schema/alternate-conflict-string.json:2: Alternate 'Alt' member 'two' can't be distinguished from member 'one'

View File

@ -1,6 +1,4 @@
# we reject alternates with multiple string-like branches
{ 'enum': 'Enum',
'data': [ 'hello', 'world' ] }
# alternate branches of 'str' type conflict with all scalar types
{ 'alternate': 'Alt',
'data': { 'one': 'str',
'two': 'Enum' } }
'two': 'int' } }

View File

@ -1,5 +1,5 @@
# we reject a nested alternate branch
{ 'alternate': 'Alt1',
'data': { 'name': 'str', 'value': 'int' } }
'data': { 'name': 'bool', 'value': 'int' } }
{ 'alternate': 'Alt2',
'data': { 'nested': 'Alt1', 'b': 'bool' } }

View File

@ -1,3 +1,3 @@
# we do not allow alternate arguments
{ 'alternate': 'Alt', 'data': { 'case1': 'int', 'case2': 'str' } }
{ 'alternate': 'Alt', 'data': { 'case1': 'int', 'case2': 'bool' } }
{ 'command': 'oops', 'data': 'Alt' }

View File

@ -6,4 +6,4 @@
# @bb: b
##
{ 'alternate': 'AorB',
'data': { 'a': 'str', 'b': 'int' } }
'data': { 'a': 'bool', 'b': 'int' } }

View File

@ -93,19 +93,22 @@
{ 'struct': 'WrapAlternate',
'data': { 'alt': 'UserDefAlternate' } }
{ 'alternate': 'UserDefAlternate',
'data': { 'udfu': 'UserDefFlatUnion', 's': 'str', 'i': 'int' } }
'data': { 'udfu': 'UserDefFlatUnion', 'e': 'EnumOne', 'i': 'int' } }
{ 'struct': 'UserDefC',
'data': { 'string1': 'str', 'string2': 'str' } }
# for testing use of 'number' within alternates
{ 'alternate': 'AltStrBool', 'data': { 's': 'str', 'b': 'bool' } }
{ 'alternate': 'AltStrNum', 'data': { 's': 'str', 'n': 'number' } }
{ 'alternate': 'AltNumStr', 'data': { 'n': 'number', 's': 'str' } }
{ 'alternate': 'AltStrInt', 'data': { 's': 'str', 'i': 'int' } }
{ 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } }
{ 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
{ 'alternate': 'AltNumEnum', 'data': { 'n': 'number', 'e': 'EnumOne' } }
{ 'alternate': 'AltEnumInt', 'data': { 'e': 'EnumOne', 'i': 'int' } }
{ 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } }
{ 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } }
# for testing use of 'str' within alternates
{ 'alternate': 'AltStrObj', 'data': { 's': 'str', 'o': 'TestStruct' } }
# for testing native lists
{ 'union': 'UserDefNativeListUnion',
'data': { 'integer': ['int'],

View File

@ -1,27 +1,31 @@
alternate AltEnumBool
tag type
case e: EnumOne
case b: bool
alternate AltEnumInt
tag type
case e: EnumOne
case i: int
alternate AltEnumNum
tag type
case e: EnumOne
case n: number
alternate AltIntNum
tag type
case i: int
case n: number
alternate AltNumEnum
tag type
case n: number
case e: EnumOne
alternate AltNumInt
tag type
case n: number
case i: int
alternate AltNumStr
tag type
case n: number
case s: str
alternate AltStrBool
alternate AltStrObj
tag type
case s: str
case b: bool
alternate AltStrInt
tag type
case s: str
case i: int
alternate AltStrNum
tag type
case s: str
case n: number
case o: TestStruct
event EVENT_A None
boxed=False
event EVENT_B None
@ -66,7 +70,7 @@ object UserDefA
alternate UserDefAlternate
tag type
case udfu: UserDefFlatUnion
case s: str
case e: EnumOne
case i: int
object UserDefB
member intb: int optional=False

View File

@ -1,3 +1,3 @@
# we reject returns if it is an alternate type
{ 'alternate': 'Alt', 'data': { 'a': 'int', 'b': 'str' } }
{ 'alternate': 'Alt', 'data': { 'a': 'int', 'b': 'bool' } }
{ 'command': 'oops', 'returns': 'Alt' }

View File

@ -42,29 +42,28 @@ static void test_clone_struct(void)
static void test_clone_alternate(void)
{
AltStrBool *b_src, *s_src, *b_dst, *s_dst;
AltEnumBool *b_src, *s_src, *b_dst, *s_dst;
b_src = g_new0(AltStrBool, 1);
b_src = g_new0(AltEnumBool, 1);
b_src->type = QTYPE_QBOOL;
b_src->u.b = true;
s_src = g_new0(AltStrBool, 1);
s_src = g_new0(AltEnumBool, 1);
s_src->type = QTYPE_QSTRING;
s_src->u.s = g_strdup("World");
s_src->u.e = ENUM_ONE_VALUE1;
b_dst = QAPI_CLONE(AltStrBool, b_src);
b_dst = QAPI_CLONE(AltEnumBool, b_src);
g_assert(b_dst);
g_assert_cmpint(b_dst->type, ==, b_src->type);
g_assert_cmpint(b_dst->u.b, ==, b_src->u.b);
s_dst = QAPI_CLONE(AltStrBool, s_src);
s_dst = QAPI_CLONE(AltEnumBool, s_src);
g_assert(s_dst);
g_assert_cmpint(s_dst->type, ==, s_src->type);
g_assert_cmpstr(s_dst->u.s, ==, s_src->u.s);
g_assert(s_dst->u.s != s_src->u.s);
g_assert_cmpint(s_dst->u.e, ==, s_src->u.e);
qapi_free_AltStrBool(b_src);
qapi_free_AltStrBool(s_src);
qapi_free_AltStrBool(b_dst);
qapi_free_AltStrBool(s_dst);
qapi_free_AltEnumBool(b_src);
qapi_free_AltEnumBool(s_src);
qapi_free_AltEnumBool(b_dst);
qapi_free_AltEnumBool(s_dst);
}
static void test_clone_native_list(void)

View File

@ -614,22 +614,26 @@ static void test_keyval_visit_alternate(void)
Error *err = NULL;
Visitor *v;
QDict *qdict;
AltNumStr *ans;
AltStrObj *aso;
AltNumInt *ani;
AltEnumBool *aeb;
/*
* Can't do scalar alternate variants other than string. You get
* the string variant if there is one, else an error.
* TODO make it work for unambiguous cases like AltEnumBool below
*/
qdict = keyval_parse("a=1,b=2", NULL, &error_abort);
qdict = keyval_parse("a=1,b=2,c=on", NULL, &error_abort);
v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
QDECREF(qdict);
visit_start_struct(v, NULL, NULL, 0, &error_abort);
visit_type_AltNumStr(v, "a", &ans, &error_abort);
g_assert_cmpint(ans->type, ==, QTYPE_QSTRING);
g_assert_cmpstr(ans->u.s, ==, "1");
qapi_free_AltNumStr(ans);
visit_type_AltNumInt(v, "a", &ani, &err);
visit_type_AltStrObj(v, "a", &aso, &error_abort);
g_assert_cmpint(aso->type, ==, QTYPE_QSTRING);
g_assert_cmpstr(aso->u.s, ==, "1");
qapi_free_AltStrObj(aso);
visit_type_AltNumInt(v, "b", &ani, &err);
error_free_or_abort(&err);
visit_type_AltEnumBool(v, "c", &aeb, &err);
error_free_or_abort(&err);
visit_end_struct(v, NULL);
visit_free(v);

View File

@ -278,11 +278,17 @@ static void test_visitor_in_number_str_keyval(TestInputVisitorData *data,
{
double res = 0, value = 3.14;
Visitor *v;
Error *err = NULL;
v = visitor_input_test_init_full(data, true, "\"3.14\"");
visit_type_number(v, NULL, &res, &error_abort);
g_assert_cmpfloat(res, ==, value);
v = visitor_input_test_init_full(data, true, "\"inf\"");
visit_type_number(v, NULL, &res, &err);
error_free_or_abort(&err);
}
static void test_visitor_in_number_str_fail(TestInputVisitorData *data,
@ -531,10 +537,10 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
g_assert_cmpint(tmp->u.i, ==, 42);
qapi_free_UserDefAlternate(tmp);
v = visitor_input_test_init(data, "'string'");
v = visitor_input_test_init(data, "'value1'");
visit_type_UserDefAlternate(v, NULL, &tmp, &error_abort);
g_assert_cmpint(tmp->type, ==, QTYPE_QSTRING);
g_assert_cmpstr(tmp->u.s, ==, "string");
g_assert_cmpint(tmp->u.e, ==, ENUM_ONE_VALUE1);
qapi_free_UserDefAlternate(tmp);
v = visitor_input_test_init(data, "{'integer':1, 'string':'str', "
@ -559,10 +565,10 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
g_assert_cmpint(wrap->alt->u.i, ==, 42);
qapi_free_WrapAlternate(wrap);
v = visitor_input_test_init(data, "{ 'alt': 'string' }");
v = visitor_input_test_init(data, "{ 'alt': 'value1' }");
visit_type_WrapAlternate(v, NULL, &wrap, &error_abort);
g_assert_cmpint(wrap->alt->type, ==, QTYPE_QSTRING);
g_assert_cmpstr(wrap->alt->u.s, ==, "string");
g_assert_cmpint(wrap->alt->u.e, ==, ENUM_ONE_VALUE1);
qapi_free_WrapAlternate(wrap);
v = visitor_input_test_init(data, "{ 'alt': {'integer':1, 'string':'str', "
@ -582,37 +588,37 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
{
Visitor *v;
Error *err = NULL;
AltStrBool *asb;
AltStrNum *asn;
AltNumStr *ans;
AltStrInt *asi;
AltEnumBool *aeb;
AltEnumNum *aen;
AltNumEnum *ans;
AltEnumInt *asi;
AltIntNum *ain;
AltNumInt *ani;
/* Parsing an int */
v = visitor_input_test_init(data, "42");
visit_type_AltStrBool(v, NULL, &asb, &err);
visit_type_AltEnumBool(v, NULL, &aeb, &err);
error_free_or_abort(&err);
qapi_free_AltStrBool(asb);
qapi_free_AltEnumBool(aeb);
v = visitor_input_test_init(data, "42");
visit_type_AltStrNum(v, NULL, &asn, &error_abort);
g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(asn->u.n, ==, 42);
qapi_free_AltStrNum(asn);
visit_type_AltEnumNum(v, NULL, &aen, &error_abort);
g_assert_cmpint(aen->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(aen->u.n, ==, 42);
qapi_free_AltEnumNum(aen);
v = visitor_input_test_init(data, "42");
visit_type_AltNumStr(v, NULL, &ans, &error_abort);
visit_type_AltNumEnum(v, NULL, &ans, &error_abort);
g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(ans->u.n, ==, 42);
qapi_free_AltNumStr(ans);
qapi_free_AltNumEnum(ans);
v = visitor_input_test_init(data, "42");
visit_type_AltStrInt(v, NULL, &asi, &error_abort);
visit_type_AltEnumInt(v, NULL, &asi, &error_abort);
g_assert_cmpint(asi->type, ==, QTYPE_QINT);
g_assert_cmpint(asi->u.i, ==, 42);
qapi_free_AltStrInt(asi);
qapi_free_AltEnumInt(asi);
v = visitor_input_test_init(data, "42");
visit_type_AltIntNum(v, NULL, &ain, &error_abort);
@ -629,26 +635,26 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
/* Parsing a double */
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrBool(v, NULL, &asb, &err);
visit_type_AltEnumBool(v, NULL, &aeb, &err);
error_free_or_abort(&err);
qapi_free_AltStrBool(asb);
qapi_free_AltEnumBool(aeb);
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrNum(v, NULL, &asn, &error_abort);
g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(asn->u.n, ==, 42.5);
qapi_free_AltStrNum(asn);
visit_type_AltEnumNum(v, NULL, &aen, &error_abort);
g_assert_cmpint(aen->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(aen->u.n, ==, 42.5);
qapi_free_AltEnumNum(aen);
v = visitor_input_test_init(data, "42.5");
visit_type_AltNumStr(v, NULL, &ans, &error_abort);
visit_type_AltNumEnum(v, NULL, &ans, &error_abort);
g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(ans->u.n, ==, 42.5);
qapi_free_AltNumStr(ans);
qapi_free_AltNumEnum(ans);
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrInt(v, NULL, &asi, &err);
visit_type_AltEnumInt(v, NULL, &asi, &err);
error_free_or_abort(&err);
qapi_free_AltStrInt(asi);
qapi_free_AltEnumInt(asi);
v = visitor_input_test_init(data, "42.5");
visit_type_AltIntNum(v, NULL, &ain, &error_abort);

View File

@ -406,12 +406,12 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
visitor_reset(data);
tmp = g_new0(UserDefAlternate, 1);
tmp->type = QTYPE_QSTRING;
tmp->u.s = g_strdup("hello");
tmp->u.e = ENUM_ONE_VALUE1;
visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
qstr = qobject_to_qstring(visitor_get(data));
g_assert(qstr);
g_assert_cmpstr(qstring_get_str(qstr), ==, "hello");
g_assert_cmpstr(qstring_get_str(qstr), ==, "value1");
qapi_free_UserDefAlternate(tmp);

View File

@ -65,11 +65,11 @@
* denote numbers, true, false or null. The special QObject input
* visitor returned by qobject_input_visitor_new_keyval() mostly hides
* this by automatically converting strings to the type the visitor
* expects. Breaks down for alternate types and type 'any', where the
* visitor's expectation isn't clear. Code visiting such types needs
* to do the conversion itself, but only when using this keyval
* visitor. Awkward. Alternate types without a string member don't
* work at all.
* expects. Breaks down for type 'any', where the visitor's
* expectation isn't clear. Code visiting 'any' needs to do the
* conversion itself, but only when using this keyval visitor.
* Awkward. Note that we carefully restrict alternate types to avoid
* similar ambiguity.
*
* Additional syntax for use with an implied key:
*