qapi: Fix to reject optional members with reserved names

check_type() fails to reject optional members with reserved names,
because it neglects to strip off the leading '*'.  Fix that.

The stripping in check_name_str() is now useless.  Drop.

Also drop the "no leading '*'" assertion, because valid_name.match()
ensures it can't fail.

Fixes: 9fb081e0b98409556d023c7193eeb68947cd1211
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-8-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 2fcaaa2..cf09fa9 100644 (file)
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -34,12 +34,10 @@ def check_name_is_str(name, info, source):
 
 
 def check_name_str(name, info, source,
-                   allow_optional=False, enum_member=False,
+                   enum_member=False,
                    permit_upper=False):
     membername = name
 
-    if allow_optional and name.startswith('*'):
-        membername = name[1:]
     # Enum members can start with a digit, because the generated C
     # code always prefixes it with the enum name
     if enum_member and membername[0].isdigit():
@@ -52,7 +50,6 @@ def check_name_str(name, info, source,
     if not permit_upper and name.lower() != name:
         raise QAPISemError(
             info, "%s uses uppercase in name" % source)
-    assert not membername.startswith('*')
 
 
 def check_defn_name_str(name, info, meta):
@@ -171,8 +168,10 @@ def check_type(value, info, source,
     # value is a dictionary, check that each member is okay
     for (key, arg) in value.items():
         key_source = "%s member '%s'" % (source, key)
+        if key.startswith('*'):
+            key = key[1:]
         check_name_str(key, info, key_source,
-                       allow_optional=True, permit_upper=permit_upper)
+                       permit_upper=permit_upper)
         if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
             raise QAPISemError(info, "%s uses reserved name" % key_source)
         check_keys(arg, info, key_source, ['type'], ['if', 'features'])

diff --git a/tests/qapi-schema/reserved-member-u.err b/tests/qapi-schema/reserved-member-u.err
index e69de29..b58e599 100644 (file)
--- a/tests/qapi-schema/reserved-member-u.err
+++ b/tests/qapi-schema/reserved-member-u.err
@@ -0,0 +1,2 @@
+reserved-member-u.json: In struct 'Oops':
+reserved-member-u.json:7: 'data' member '*u' uses reserved name

diff --git a/tests/qapi-schema/reserved-member-u.json b/tests/qapi-schema/reserved-member-u.json
index 15005ab..2bfb8f5 100644 (file)
--- a/tests/qapi-schema/reserved-member-u.json
+++ b/tests/qapi-schema/reserved-member-u.json
@@ -4,5 +4,4 @@
 # This is true even for non-unions, because it is possible to convert a
 # struct to flat union while remaining backwards compatible in QMP.
 # TODO - we could munge the member name to 'q_u' to avoid the collision
-# BUG: not rejected
 { 'struct': 'Oops', 'data': { '*u': 'str' } }

diff --git a/tests/qapi-schema/reserved-member-u.out b/tests/qapi-schema/reserved-member-u.out
index 6a37055..e69de29 100644 (file)
--- a/tests/qapi-schema/reserved-member-u.out
+++ b/tests/qapi-schema/reserved-member-u.out
@@ -1,14 +0,0 @@
-module ./builtin
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module reserved-member-u.json
-object Oops
-    member u: str optional=True