[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/19] tests/qapi-schema: Demonstrate insufficie
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 05/19] tests/qapi-schema: Demonstrate insufficient 'if' checking |
Date: |
Mon, 23 Sep 2019 13:55:11 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 9/14/19 10:34 AM, Markus Armbruster wrote:
>> Cover invalid 'if' in struct members, features, union and alternate
>> branches. Four out of four are broken. Mark FIXME.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
> Embarrassing. But the fact that you're pointing them out presumably
> means that you fix it later in the series ;)
Yes:
[PATCH 09/19] qapi: Remove null from schema language
[PATCH 12/19] qapi: Fix missing 'if' checks in struct, union, alternate 'data'
>> +++ b/tests/qapi-schema/features-if-invalid.json
>> @@ -0,0 +1,5 @@
>> +# Cover feature with invalid 'if'
>> +# FIXME not rejected, misinterpreded as unconditional
>
> misinterpreted
>
> With the typo fix,
>
> Reviewed-by: Eric Blake <address@hidden>
Thanks!
>> +++ b/tests/qapi-schema/struct-member-if-invalid.json
>> @@ -0,0 +1,4 @@
>> +# Cover member with invalid 'if'
>> +# FIXME not rejected, would generate '#if True\n'
>
> Which might actually compile, depending on what else is present in
> various headers! But certainly not what was intended.
>
>> +++ b/tests/qapi-schema/union-branch-if-invalid.json
>> @@ -0,0 +1,7 @@
>> +# Cover branch with invalid 'if'
>> +# FIXME not rejected, would generate '#if \n'
>> +{ 'enum': 'Branches', 'data': ['branch1'] }
>> +{ 'struct': 'Stru', 'data': { 'member': 'str' } }
>> +{ 'union': 'Uni',
>> + 'base': { 'tag': 'Branches' }, 'discriminator': 'tag',
>> + 'data': { 'branch1': { 'type': 'Stru', 'if': [''] } } }
>
> So you're pointing out a difference between an empty string and a string
> not containing a C macro name (possibly because later patches will give
> them different error messages).
Not sure I got this comment.
- [Qemu-devel] [PATCH 00/19] qapi: Frontend fixes and cleanups, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 03/19] tests/qapi-schema: Demonstrate misleading optional tag error, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 14/19] qapi: Simplify check_keys(), Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 12/19] qapi: Fix missing 'if' checks in struct, union, alternate 'data', Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 10/19] qapi: Fix broken discriminator error messages, Markus Armbruster, 2019/09/14
- [Qemu-devel] [PATCH 02/19] tests/qapi-schema: Delete two redundant tests, Markus Armbruster, 2019/09/14