On 9/10/19 1:37 AM, Markus Armbruster wrote: > The specification claims "Each expression that isn't an include > directive may be preceded by a documentation block", but the code also > rejects them for pragma directives. The code is correct. Fix the > specification. > > The specification reserves member names starting with 'has_', but the > code also reserves name 'u'. Fix the specification.
Reservation of 'u' was done in 5e59baf9 (and claimed we could add a
munge to q_u in the future if we ever needed a name 'u' after all).
>
> The specification claims "The string 'max' is not allowed as an enum
> value". Untrue. Fix the specification. While there, delete the
> naming advice, because it's redundant with the naming rules in section
> "Schema overview"
Used to be true; missed when commit 7fb1cf16 got rid of the collision.
>
> The specification claims "No branch of the union can be named 'max',
> as this would collide with the implicit enum". Untrue. Fix the
> specification.
Fixed around the same time (although I didn't check if it was in the
same commit)
>
> The specification claims "It is not allowed to name an event 'MAX',
> since the generator also produces a C enumeration of all event names
> with a generated _MAX value at the end." Untrue. Fix the
> specification.
And similar comment.
I don't know if you want to do exact commit ids where all of these doc
problems were introduced (because of code patches that lifted the
limitations).
>
> The specification claims "All branches of the union must be complex
> types", but the code permits only struct types. The code is correct.
> Fix the specification.
>
> The specification claims a command's return type "must be the string
> name of a complex or built-in type, a one-element array containing the
> name of a complex or built-in type" unless the command is in pragma
> 'returns-whitelist'. The code does not permit built-in types. Fix
> the specification.
Umm:
qapi/migration.json:{ 'command': 'query-migrate-cache-size', 'returns':
'int' }
I don't know if we use an array of a built-in-type, but we definitely
have unfortunate commands that return a non-JSON-object. [1]
> A flat union definition avoids nesting on the wire, and specifies a
> set of common members that occur in all variants of the union. The
> 'base' key must specify either a type name (the type must be a
> struct, not a union), or a dictionary representing an anonymous type.
> -All branches of the union must be complex types, and the top-level
> +All branches of the union must be struct types, and the top-level
We have hit cases where it might have been nicer to permit a flat union
whose branch is itself another flat union. But until we actually code
that up to work, this is accurate.
> @@ -578,8 +578,8 @@ The 'returns' member describes what will appear in the
> "return" member
> of a Client JSON Protocol reply on successful completion of a command.
> The member is optional from the command declaration; if absent, the
> "return" member will be an empty dictionary. If 'returns' is present,
> -it must be the string name of a complex or built-in type, a
> -one-element array containing the name of a complex or built-in type.
> +it must be the string name of a complex type, or a
> +one-element array containing the name of a complex type.
> To return anything else, you have to list the command in pragma
> 'returns-whitelist'. If you do this, the command cannot be extended
> to return additional information in the future. Use of
[1] Aha - it's 'returns-whitelist' that makes the difference. Okay,
your wording change here makes sense: a built-in is NOT permitted UNLESS
you whitelist it.
Summary: you may want to improve the commit message with git
archaeology, but the wording changes themselves make sense.
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
