Re: [Qemu-devel] [PATCH RFC v3 32/32] qapi-introspect: Hide type names

[Eric Blake]

On 08/04/2015 09:58 AM, Markus Armbruster wrote:
> To eliminate the temptation for clients to look up types by name
> (which are not ABI), replace all type names by meaningless strings.
> 
> Reduces output of query-schema by 13 out of 85KiB.

I'm starting to be more in favor of this patch, both for ABI reasons and
for size shavings.  It definitely looks better than in v2, where munged
names are just an arbitrary number, and where builtins are not munged.

> 
> TODO Either generate comments with the true type names, or provide an
> option to generate without type name hiding.

See also my comments on 30/32 about whether we should mask array types
differently (and yes, the lack of comments made it a bit harder to chase
down types for my commentary in that mail).

> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  docs/qapi-code-gen.txt     | 14 +++++++-------
>  scripts/qapi-introspect.py | 24 +++++++++++++++++++++---
>  2 files changed, 28 insertions(+), 10 deletions(-)
> 

If we don't do anything to array names, then:
Reviewed-by: Eric Blake <address@hidden>

If we DO touch array names, I suspect this will look a lot different,
and have to drop R-b for v4.

> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index ed04770..3a78cf4 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -868,13 +868,13 @@ Example:
>  [Uninteresting stuff omitted...]
>  
>      const char example_qmp_schema_json[] = "["
> -        "{ \"arg-type\": \":empty\", \"name\": \"MY_EVENT\", \"meta-type\": 
> \"event\" }, "
> -        "{ \"meta-type\": \"builtin\", \"name\": \"int\", \"json-type\": 
> \"int\" }, "
> -        "{ \"meta-type\": \"builtin\", \"name\": \"str\", \"json-type\": 
> \"string\" }, "
> -        "{ \"meta-type\": \"object\", \"name\": \":empty\", \"members\": [ ] 
> }, "
> -        "{ \"meta-type\": \"object\", \"name\": \":obj-my-command-arg\", 
> \"members\": [{ \"type\": \"UserDefOne\", \"name\": \"arg1\" } ] }, "
> -        "{ \"meta-type\": \"object\", \"name\": \"UserDefOne\", \"members\": 
> [{ \"type\": \"int\", \"name\": \"integer\" }, { \"type\": \"str\", \"name\": 
> \"string\" } ] }, "
> -        "{ \"arg-type\": \":obj-my-command-arg\", \"ret-type\": 
> \"UserDefOne\", \"name\": \"my-command\", \"meta-type\": \"command\" } ]";
> +        "{ \"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": 
> \"MY_EVENT\" }, "
> +        "{ \"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": 
> \"my-command\", \"ret-type\": \"2\" }, "
> +        "{ \"members\": [ ], \"meta-type\": \"object\", \"name\": \"0\" }, "
> +        "{ \"members\": [{ \"name\": \"arg1\", \"type\": \"2\" } ], 
> \"meta-type\": \"object\", \"name\": \"1\" }, "
> +        "{ \"members\": [{ \"name\": \"integer\", \"type\": \"int\" }, { 
> \"name\": \"string\", \"type\": \"str\" } ], \"meta-type\": \"object\", 
> \"name\": \"2\" }, "
> +        "{ \"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": 
> \"int\" }, "
> +        "{ \"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": 
> \"str\" } ]";

Some churn here once you fix the sorting in 30.

> +
> +    def _name(self, name):
> +        if name not in self.name_map:
> +            n = len(self.name_map)
> +            self.name_map[name] = '%s' % n

When string-izing an integer, isn't it better to use:

self.name_map[name] = '%d' % len(self.name_map)

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

signature.asc
Description: OpenPGP digital signature