Eric Blake <ebl...@redhat.com> writes:
> 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.
Yes, much easier to read, and smaller, too. Review pays :)
>> 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 <arm...@redhat.com>
>> ---
>> 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 <ebl...@redhat.com>
>
> 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)
Yes, will change.
Thanks!
