[Qemu-devel] [PATCH] fixup! qapi: Tighten the regex on valid names

[Eric Blake <eblake@redhat.com>]

[Replace the old commit message with this:]

qapi: Tighten the regex on valid names

We already documented that qapi names should match specific
patterns (such as starting with a letter unless it was an enum
value or a downstream extension).  Tighten that from a suggestion
into a hard requirement, which frees up names beginning with a
single underscore for qapi internal usage.

The tighter regex doesn't forbid everything insane that a user
could provide (for example, a user could name a type 'Foo-lookup'
to collide with the generated 'Foo_lookup[]' for an enum 'Foo'),
but does a good job at protecting the most obvious uses, and
also happens to reserve single leading underscore for later use.

The handling of enum values starting with a digit is tricky:
commit 9fb081e introduced a subtle bug by using c_name() on
a munged value, which would allow an enum to include the
member 'q-int' in spite of our reservation.  Furthermore,
munging with a leading '_' would fail our tighter regex.  So
fix it by only munging for leading digits (which are never
ticklish in c_name()) and by using a different prefix (I
picked 'D', although any letter should do).

Add new tests, reserved-member-underscore and reserved-enum-q,
to demonstrate the tighter checking.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt                 | 11 +++++------
 scripts/qapi.py                        |  4 ++--
 tests/Makefile                         |  1 +
 tests/qapi-schema/reserved-enum-q.err  |  1 +
 tests/qapi-schema/reserved-enum-q.exit |  1 +
 tests/qapi-schema/reserved-enum-q.json |  4 ++++
 tests/qapi-schema/reserved-enum-q.out  |  0
 7 files changed, 14 insertions(+), 8 deletions(-)
 create mode 100644 tests/qapi-schema/reserved-enum-q.err
 create mode 100644 tests/qapi-schema/reserved-enum-q.exit
 create mode 100644 tests/qapi-schema/reserved-enum-q.json
 create mode 100644 tests/qapi-schema/reserved-enum-q.out

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index ec225d1..32cc68f 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -119,12 +119,11 @@ tracking optional fields.
 Any name (command, event, type, field, or enum value) beginning with
 "x-" is marked experimental, and may be withdrawn or changed
 incompatibly in a future release.  All names must begin with a letter,
-and contain only ASCII letters, digits, dash, and underscore, where
-dash and underscore cannot occur consecutively.  There are two
-exceptions: enum values may start with a digit, and any extensions
-added by downstream vendors should start with a prefix matching
-"__RFQDN_" (for the reverse-fully-qualified-domain-name of the
-vendor), even if the rest of the name uses dash (example:
+and contain only ASCII letters, digits, dash, and underscore.  There
+are two exceptions: enum values may start with a digit, and any
+extensions added by downstream vendors should start with a prefix
+matching "__RFQDN_" (for the reverse-fully-qualified-domain-name of
+the vendor), even if the rest of the name uses dash (example:
 __com.redhat_drive-mirror).  Names beginning with 'q_' are reserved
 for the generator: QMP names that resemble C keywords or other
 problematic strings will be munged in C to use this prefix.  For
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e6d014b..5b6d18d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -356,8 +356,8 @@ def discriminator_find_enum_define(expr):
 # Names must be letters, numbers, -, and _.  They must start with letter,
 # except for downstream extensions which must start with __RFQDN_.
 # Dots are only valid in the downstream extension prefix.
-valid_name = re.compile('^(__[a-zA-Z][a-zA-Z0-9.]*_)?'
-                        '[a-zA-Z][a-zA-Z0-9]*([_-][a-zA-Z0-9]+)*$')
+valid_name = re.compile('^(__[a-zA-Z0-9.-]+_)?'
+                        '[a-zA-Z][a-zA-Z0-9_-]*$')


 def check_name(expr_info, source, name, allow_optional=False,
diff --git a/tests/Makefile b/tests/Makefile
index b70d246..a3f63f5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -317,6 +317,7 @@ qapi-schema += redefined-command.json
 qapi-schema += redefined-event.json
 qapi-schema += redefined-type.json
 qapi-schema += reserved-command-q.json
+qapi-schema += reserved-enum-q.json
 qapi-schema += reserved-member-has.json
 qapi-schema += reserved-member-q.json
 qapi-schema += reserved-member-u.json
diff --git a/tests/qapi-schema/reserved-enum-q.err b/tests/qapi-schema/reserved-enum-q.err
new file mode 100644
index 0000000..e1c3480
--- /dev/null
+++ b/tests/qapi-schema/reserved-enum-q.err
@@ -0,0 +1 @@
+tests/qapi-schema/reserved-enum-q.json:4: Member of enum 'Foo' uses invalid name 'q-Unix'
diff --git a/tests/qapi-schema/reserved-enum-q.exit b/tests/qapi-schema/reserved-enum-q.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/reserved-enum-q.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/reserved-enum-q.json b/tests/qapi-schema/reserved-enum-q.json
new file mode 100644
index 0000000..3593a76
--- /dev/null
+++ b/tests/qapi-schema/reserved-enum-q.json
@@ -0,0 +1,4 @@
+# C entity name collision
+# We reject names like 'q-unix', because they can collide with the mangled
+# name for 'unix' in generated C.
+{ 'enum': 'Foo', 'data': [ 'unix', 'q-Unix' ] }
diff --git a/tests/qapi-schema/reserved-enum-q.out b/tests/qapi-schema/reserved-enum-q.out
new file mode 100644
index 0000000..e69de29
-- 
2.4.3