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 <arm...@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 <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
