Re: [Qemu-devel] [PATCH v5 22/28] qapi: Whitelist commands that don't return dictionary

[Eric Blake]

On 03/27/2015 03:11 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> ...or an array of dictionaries.  Although we have to cater to
>> existing commands, returning a non-dictionary means the command
>> is not extensible (no new name/value pairs can be added if more
>> information must be returned in parallel).  By making the
>> whitelist explicit, any new command that falls foul of this
>> practice will have to be self-documenting, which will encourage
>> developers to either justify the action or rework the design to
>> use a dictionary after all.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> ---

> Since there's just one whitelist, all schemata share it, and that means
> it's too permissive for each of them.  Sloppy, but good enough.
> 
> If the sloppiness bothers us, here are two alternatives:

Doesn't bother me enough to use either alternative, but I _will_ update
the commit message to at least remind someone of the potential for
tightening things, should the need arise in the future.

> 
> * Program takes the whitelist as argument, say
> 
>     scripts/qapi-commands.py --legacy-returns qmp-legacy-returns ...

Early exit (generator fails if you don't whitelist), but noisy (have to
touch a Makefile in addition to the .json schema).

> 
> * Leave enforcing to C
> 
>   If a command 'frobnicate' returns a non-dictionary, generate something
>   like
> 
>       #ifndef FROBNICATE_LEGACY_RETURN_OK
>       #error Command 'frobnicate' should return a dictionary
>       #endif
> 
>   Then manually define the macros necessary to keep the current use
>   working in a suitable header.

Late exit (failure does not occur at python generation time, but only at
C compilation time) - and the point of this series was to promote
several late exits into early exits.  But slightly cleaner than having
to touch Makefiles to add in the whitelist.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature