Re: [Qemu-devel] [PATCH 05/19] tests/qapi-schema: Demonstrate insufficient 'if' checking

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1634 bytes --]

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 <armbru@redhat.com>
> ---

Embarrassing. But the fact that you're pointing them out presumably
means that you fix it later in the series ;)

> +++ 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 <eblake@redhat.com>

> +++ 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).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]