[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 08/28] qapi: Support flat unions tag values with leading digi
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 08/28] qapi: Support flat unions tag values with leading digit |
Date: |
Tue, 23 Mar 2021 22:07:55 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Markus Armbruster <armbru@redhat.com> writes:
> John Snow <jsnow@redhat.com> writes:
>
>> On 3/23/21 5:40 AM, Markus Armbruster wrote:
>>> Flat union tag values get checked twice: as enum member name, and as
>>> union branch name. The former accepts leading digits, the latter
>>> doesn't. The restriction feels arbitrary. Skip the latter check.
>>>
>>> This can expose c_name() to input it can't handle: a name starting
>>> with a digit. Improve it to return a valid C identifier for any
>>> input.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>> Anything in particular inspire this?
>
> Just a desire for keeping things simple. "Any enum type works as
> discriminator" is simpler than "any enum works, but branches
> corresponding to enum values starting with a digit cannot have members".
> Let me elaborate.
>
> This works:
>
> {'enum': 'Enu', 'data': ['0', 'eins', '2']}
> {'struct': 'St', 'data': {'s': 'str'}}
> {'union': 'Uni',
> 'base': {'type': 'Enu'},
> 'discriminator': 'type',
> 'data': {
> 'eins': 'St'}}
>
> But if you change the last line to
>
> '0': 'St'}}
>
> you get told off:
>
> scripts/qapi-gen.py: /dev/stdin: In union 'Uni':
> /dev/stdin:3: 'data' member '0' has an invalid name
Improved commit message:
qapi: Permit flat union members for any tag value
Flat union branch names match the tag enum's member names. Omitted
branches default to "no members for this tag value".
Branch names starting with a digit get rejected like "'data' member
'0' has an invalid name". However, omitting the branch works.
This is because flat union tag values get checked twice: as enum
member name, and as union branch name. The former accepts leading
digits, the latter doesn't.
Branches whose names start with a digit therefore cannot have members.
Feels wrong. Get rid of the restriction by skipping the latter check.
This can expose c_name() to input it can't handle: a name starting
with a digit. Improve it to return a valid C identifier for any
input.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
[...]
- [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, (continued)
[PATCH 08/28] qapi: Support flat unions tag values with leading digit, Markus Armbruster, 2021/03/23
[PATCH 07/28] qapi: Fix to reject optional members with reserved names, Markus Armbruster, 2021/03/23
[PATCH 26/28] qapi: Enforce struct member naming rules, Markus Armbruster, 2021/03/23
[PATCH 15/28] tests/qapi-schema: Rename redefined-builtin to redefined-predefined, Markus Armbruster, 2021/03/23
[PATCH 22/28] qapi: Prepare for rejecting underscore in command and member names, Markus Armbruster, 2021/03/23