From: Eric Blake Date: Wed, 18 Nov 2015 08:52:56 +0000 (-0700) Subject: qapi: Tighten the regex on valid names X-Git-Tag: v2.7.1~689^2~19 X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=59a92feedc6927e0e1ff87fdaccfb4dd42ad4c84;p=mirror_qemu.git 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 Message-Id: <1447836791-369-22-git-send-email-eblake@redhat.com> Message-Id: <1447883135-18020-1-git-send-email-eblake@redhat.com> [Eric's fixup squashed in] Signed-off-by: Markus Armbruster --- diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index ceb9a782d0..32cc68fff1 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -118,17 +118,17 @@ 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. Downstream vendors may add -extensions; such extensions should begin 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). Other than downstream extensions (with -leading underscore and the use of dots), all names should begin with a -letter, and contain only ASCII letters, digits, dash, and underscore. -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 example, a field named "default" in -qapi becomes "q_default" in the generated C code. +incompatibly in a future release. All names must begin with a letter, +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 +example, a field named "default" in qapi becomes "q_default" in the +generated C code. In the rest of this document, usage lines are given for each expression type, with literal strings written in lower case and diff --git a/scripts/qapi.py b/scripts/qapi.py index d2ce9b3b19..3e5caa8193 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -353,9 +353,11 @@ def discriminator_find_enum_define(expr): return find_enum(discriminator_type) -# FIXME should enforce "other than downstream extensions [...], all -# names should begin with a letter". -valid_name = re.compile('^[a-zA-Z_][a-zA-Z0-9_.-]*$') +# 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-Z0-9.-]+_)?' + '[a-zA-Z][a-zA-Z0-9_-]*$') def check_name(expr_info, source, name, allow_optional=False, @@ -374,8 +376,8 @@ def check_name(expr_info, source, name, allow_optional=False, % (source, name)) # Enum members can start with a digit, because the generated C # code always prefixes it with the enum name - if enum_member: - membername = '_' + membername + if enum_member and membername[0].isdigit(): + membername = 'D' + membername # Reserve the entire 'q_' namespace for c_name() if not valid_name.match(membername) or \ c_name(membername, False).startswith('q_'): diff --git a/tests/Makefile b/tests/Makefile index 6518ef9986..2f6b2340ef 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -318,9 +318,11 @@ 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 +qapi-schema += reserved-member-underscore.json qapi-schema += reserved-type-kind.json qapi-schema += reserved-type-list.json qapi-schema += returns-alternate.json diff --git a/tests/qapi-schema/reserved-enum-q.err b/tests/qapi-schema/reserved-enum-q.err new file mode 100644 index 0000000000..e1c3480ee2 --- /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 0000000000..d00491fd7e --- /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 0000000000..3593a765ea --- /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 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/reserved-member-underscore.err b/tests/qapi-schema/reserved-member-underscore.err new file mode 100644 index 0000000000..65ff0da8ce --- /dev/null +++ b/tests/qapi-schema/reserved-member-underscore.err @@ -0,0 +1 @@ +tests/qapi-schema/reserved-member-underscore.json:4: Member of 'data' for struct 'Oops' uses invalid name '_oops' diff --git a/tests/qapi-schema/reserved-member-underscore.exit b/tests/qapi-schema/reserved-member-underscore.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/reserved-member-underscore.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/reserved-member-underscore.json b/tests/qapi-schema/reserved-member-underscore.json new file mode 100644 index 0000000000..4a3a017638 --- /dev/null +++ b/tests/qapi-schema/reserved-member-underscore.json @@ -0,0 +1,4 @@ +# C member name collision +# We reject use of a single leading underscore in all names (names must +# begin with a letter or a downstream extension double-underscore prefix). +{ 'struct': 'Oops', 'data': { '_oops': 'str' } } diff --git a/tests/qapi-schema/reserved-member-underscore.out b/tests/qapi-schema/reserved-member-underscore.out new file mode 100644 index 0000000000..e69de29bb2