Re: [Qemu-devel] [PATCH 09/19] qapi: Remove null from schema language

[Eric Blake <eblake@redhat.com>]

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

On 9/14/19 10:34 AM, Markus Armbruster wrote:
> We represent the parse tree as OrderedDict.  We fetch optional dict
> members with .get().  So far, so good.
> 
> We represent null literals as None.  .get() returns None both for
> "absent" and for "present, value is the null literal".  Uh-oh.
> 
> Test features-if-invalid exposes this bug: "'if': null" is
> misinterpreted as absent "if".
> 
> We added null to the schema language to "allow [...] an explicit
> default value" (commit e53188ada5 "qapi: Allow true, false and null in
> schema json", v2.4.0).  Hasn't happened; null is still unused except
> as generic invalid value in tests/.

Might happen as a way to default a '*value':'str', but we can deal with
that at the point where someone actually justifies a need for a way to
express a default like that.

> 
> To fix, we'd have to replace .get() by something more careful, or
> represent null differently.  Feasible, but we got more and bigger fish
> to fry right now.  Remove the null literal from the schema language.

Fair enough.

> Replace null in tests by another invalid value.
> 
> Test features-if-invalid now behaves as it should.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/tests/qapi-schema/features-if-invalid.json
> @@ -1,5 +1,4 @@
>  # Cover feature with invalid 'if'
> -# FIXME not rejected, misinterpreded as unconditional

Obvious conflict resolution with the typo fix earlier in the series.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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 --]