diff mbox

[RFC,v4,32/32] qapi-introspect: Hide type names

Message ID 1441290623-13631-33-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Sept. 3, 2015, 2:30 p.m. UTC
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.

As a debugging aid, provide option -u to suppress the hiding.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/qapi-code-gen.txt     | 36 +++++++++++++++++-------------------
 qapi/introspect.json       | 11 +++--------
 scripts/qapi-introspect.py | 41 +++++++++++++++++++++++++++++++++++------
 3 files changed, 55 insertions(+), 33 deletions(-)

Comments

Eric Blake Sept. 4, 2015, 2:07 p.m. UTC | #1
On 09/03/2015 08:30 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.
> 
> As a debugging aid, provide option -u to suppress the hiding.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/qapi-code-gen.txt     | 36 +++++++++++++++++-------------------
>  qapi/introspect.json       | 11 +++--------
>  scripts/qapi-introspect.py | 41 +++++++++++++++++++++++++++++++++++------
>  3 files changed, 55 insertions(+), 33 deletions(-)
> 
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index a079b51..94fc296 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -516,13 +516,16 @@ additional variant members depending on the value of meta-type.
>  Each SchemaInfo object describes a wire ABI entity of a certain
>  meta-type: a command, event or one of several kinds of type.
>  
> -SchemaInfo for entities defined in the QAPI schema have the same name
> -as in the schema.  This is the case for all commands and events, and
> -most types.
> +SchemaInfo for commands and events have the same name as in the QAPI
> +schema

Missing a trailing '.'

>  
>  Command and event names are part of the wire ABI, but type names are
> -not.  Therefore, looking up a type by its name in the QAPI schema is
> -wrong.  Look up the command or event, then follow references by name.
> +not.  Therefore, the SchemaInfo for types have auto-generated
> +meaningless names.  For readability, the examples in this document use
> +meaningful type names instead.
> +
> +To examine a type, start with a the command or event using it, then

s/a the/the/

> +follow references by name.
>  

> @@ -1053,13 +1051,13 @@ Example:
>  [Uninteresting stuff omitted...]
>  
>      const char example_qmp_schema_json[] = "["
> -        "{\"arg-type\": \":empty\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
> +        "{\"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
> +        "{\"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"2\"}, "

I thought you said that this document would use the meaningful type
names?  Oh, I see - the earlier examples (not touched by this commit) DO
use meaningful names; while _this_ example uses the munged names. I
don't know if any additional wording would help.

Would it be worth documenting -u as a usage argument for
qapi-introspect.py?  [For that matter, should we document what -m for
middle-mode does for qapi-commands.py? But as a separate patch]

I still think that masking arrays as "[123]" is nicer, but that can be a
followup patch (especially since I've already posted it in one of my
numerous RFC followups, which I'll have to rebase anyways)

Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster Sept. 4, 2015, 3:52 p.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 09/03/2015 08:30 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.
>> 
>> As a debugging aid, provide option -u to suppress the hiding.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  docs/qapi-code-gen.txt     | 36 +++++++++++++++++-------------------
>>  qapi/introspect.json       | 11 +++--------
>>  scripts/qapi-introspect.py | 41 +++++++++++++++++++++++++++++++++++------
>>  3 files changed, 55 insertions(+), 33 deletions(-)
>> 
>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>> index a079b51..94fc296 100644
>> --- a/docs/qapi-code-gen.txt
>> +++ b/docs/qapi-code-gen.txt
>> @@ -516,13 +516,16 @@ additional variant members depending on the
>> value of meta-type.
>>  Each SchemaInfo object describes a wire ABI entity of a certain
>>  meta-type: a command, event or one of several kinds of type.
>>  
>> -SchemaInfo for entities defined in the QAPI schema have the same name
>> -as in the schema.  This is the case for all commands and events, and
>> -most types.
>> +SchemaInfo for commands and events have the same name as in the QAPI
>> +schema
>
> Missing a trailing '.'

Will fix.

>>  Command and event names are part of the wire ABI, but type names are
>> -not.  Therefore, looking up a type by its name in the QAPI schema is
>> -wrong.  Look up the command or event, then follow references by name.
>> +not.  Therefore, the SchemaInfo for types have auto-generated
>> +meaningless names.  For readability, the examples in this document use
>> +meaningful type names instead.
>> +
>> +To examine a type, start with a the command or event using it, then
>
> s/a the/the/

Will fix.

>> +follow references by name.
>>  
>
>> @@ -1053,13 +1051,13 @@ Example:
>>  [Uninteresting stuff omitted...]
>>  
>>      const char example_qmp_schema_json[] = "["
>> -        "{\"arg-type\": \":empty\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
>> +        "{\"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
>> +        "{\"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"2\"}, "
>
> I thought you said that this document would use the meaningful type
> names?  Oh, I see - the earlier examples (not touched by this commit) DO
> use meaningful names; while _this_ example uses the munged names. I
> don't know if any additional wording would help.

Different section.  I'll change the wording there

    meaningless names.  For readability, the examples in this document use
    meaningful type names instead.

to say "this section" instead of "this document".

> Would it be worth documenting -u as a usage argument for
> qapi-introspect.py?  [For that matter, should we document what -m for
> middle-mode does for qapi-commands.py? But as a separate patch]

None of the options are documented outside the source, and even the
source doesn't really explain.  It's just a development tool.  I guess
adding --help would be nice.  Beyond that, returns diminish rapidly.

> I still think that masking arrays as "[123]" is nicer, but that can be a
> followup patch (especially since I've already posted it in one of my
> numerous RFC followups, which I'll have to rebase anyways)
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
diff mbox

Patch

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a079b51..94fc296 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -516,13 +516,16 @@  additional variant members depending on the value of meta-type.
 Each SchemaInfo object describes a wire ABI entity of a certain
 meta-type: a command, event or one of several kinds of type.
 
-SchemaInfo for entities defined in the QAPI schema have the same name
-as in the schema.  This is the case for all commands and events, and
-most types.
+SchemaInfo for commands and events have the same name as in the QAPI
+schema
 
 Command and event names are part of the wire ABI, but type names are
-not.  Therefore, looking up a type by its name in the QAPI schema is
-wrong.  Look up the command or event, then follow references by name.
+not.  Therefore, the SchemaInfo for types have auto-generated
+meaningless names.  For readability, the examples in this document use
+meaningful type names instead.
+
+To examine a type, start with a the command or event using it, then
+follow references by name.
 
 QAPI schema definitions not reachable that way are omitted.
 
@@ -554,8 +557,7 @@  object type without members.  The event may not have a data member on
 the wire then.
 
 Each command or event defined with dictionary-valued 'data' in the
-QAPI schema implicitly defines an object type called ":obj-NAME-arg",
-where NAME is the command or event's name.
+QAPI schema implicitly defines an object type.
 
 Example: the SchemaInfo for EVENT_C from section Events
 
@@ -609,12 +611,9 @@  Note that base types are "flattened": its members are included in the
 
 A simple union implicitly defines an enumeration type for its implicit
 discriminator (called "type" on the wire, see section Union types).
-Such a type's name is made by appending "Kind" to the simple union's
-name.
 
 A simple union implicitly defines an object type for each of its
-variants.  The type's name is ":obj-NAME-wrapper", where NAME is the
-name of the name of the variant's type.
+variants.
 
 Example: the SchemaInfo for simple union BlockdevOptions from section
 Union types
@@ -645,8 +644,7 @@  Example: the SchemaInfo for BlockRef from section Alternate types
 
 The SchemaInfo for an array type has meta-type "array", and variant
 member "element-type", which names the array's element type.  Array
-types are implicitly defined.  An array type's name is made by
-appending "List" to its element type's name.
+types are implicitly defined.
 
 Example: the SchemaInfo for ['str']
 
@@ -1053,13 +1051,13 @@  Example:
 [Uninteresting stuff omitted...]
 
     const char example_qmp_schema_json[] = "["
-        "{\"arg-type\": \":empty\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
+        "{\"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\"}, "
-        "{\"members\": [], \"meta-type\": \"object\", \"name\": \":empty\"}, "
-        "{\"members\": [{\"name\": \"arg1\", \"type\": \"UserDefOne\"}], \"meta-type\": \"object\", \"name\": \":obj-my-command-arg\"}, "
-        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"UserDefOne\"}, "
-        "{\"arg-type\": \":obj-my-command-arg\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"UserDefOne\"}]";
+        "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}]";
     $ cat qapi-generated/example-qmp-introspect.h
 [Uninteresting stuff omitted...]
 
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 171baba..46e78a1 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -70,17 +70,12 @@ 
 # @SchemaInfo
 #
 # @name: the entity's name, inherited from @base.
-# Entities defined in the QAPI schema have the name defined in the schema.
-# Implicitly defined entities have generated names.  See
-# docs/qapi-code-gen.txt section "Client JSON Protocol introspection"
-# for details.
+# Commands and events have the name defined in the QAPI schema.
+# Unlike command and event names, type names are not part of the wire
+# ABI.  Consequently, type names are meaningless strings here.
 #
 # All references to other SchemaInfo are by name.
 #
-# Command and event names are part of the wire ABI, but type names are
-# not.  Therefore, looking up a type by "well-known" name is wrong.
-# Look up the command or event, then follow the references.
-#
 # @meta-type: the entity's meta type, inherited from @base.
 #
 # Additional members depend on the value of @meta-type.
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 2f6a4c6..9cc101d 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -37,32 +37,37 @@  def to_c_string(string):
     return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
 
 class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
-    def __init__(self):
+    def __init__(self, unmask):
+        self.unmask = unmask
         self.defn = None
         self.decl = None
         self.schema = None
         self.jsons = None
         self.used_types = None
+        self.name_map = None
 
     def visit_begin(self, schema):
         self.schema = schema
         self.jsons = []
         self.used_types = []
+        self.name_map = {}
         return QAPISchemaType   # don't visit types for now
 
     def visit_end(self):
         # visit the types that are actually used
+        jsons = self.jsons
+        self.jsons = []
         for typ in self.used_types:
             typ.visit(self)
-        self.jsons.sort()
         # generate C
         # TODO can generate awfully long lines
+        jsons.extend(self.jsons)
         name = prefix + 'qmp_schema_json'
         self.decl = mcgen('''
 extern const char %(c_name)s[];
 ''',
                           c_name=c_name(name))
-        lines = to_json(self.jsons).split('\n')
+        lines = to_json(jsons).split('\n')
         c_string = '\n    '.join([to_c_string(line) for line in lines])
         self.defn = mcgen('''
 const char %(c_name)s[] = %(c_string)s;
@@ -72,6 +77,14 @@  const char %(c_name)s[] = %(c_string)s;
         self.schema = None
         self.jsons = None
         self.used_types = None
+        self.name_map = None
+
+    def _name(self, name):
+        if self.unmask:
+            return name
+        if name not in self.name_map:
+            self.name_map[name] = '%d' % len(self.name_map)
+        return self.name_map[name]
 
     def _use_type(self, typ):
         # Map the various integer types to plain int
@@ -83,9 +96,16 @@  const char %(c_name)s[] = %(c_string)s;
         # Add type to work queue if new
         if typ not in self.used_types:
             self.used_types.append(typ)
-        return typ.name
+        # Clients should examine commands and events, not types.  Hide
+        # type names to reduce the temptation.  Also saves a few
+        # characters.
+        if isinstance(typ, QAPISchemaBuiltinType):
+            return typ.name
+        return self._name(typ.name)
 
     def _gen_json(self, name, mtype, obj):
+        if mtype != 'command' and mtype != 'event' and mtype != 'builtin':
+            name = self._name(name)
         obj['name'] = name
         obj['meta-type'] = mtype
         self.jsons.append(obj)
@@ -137,7 +157,16 @@  const char %(c_name)s[] = %(c_string)s;
         arg_type = arg_type or self.schema.the_empty_object_type
         self._gen_json(name, 'event', {'arg-type': self._use_type(arg_type)})
 
-(input_file, output_dir, do_c, do_h, prefix, dummy) = parse_command_line()
+# Debugging aid: unmask QAPI schema's type names 
+# We normally mask them, because they're not QMP wire ABI
+opt_unmask = False
+
+(input_file, output_dir, do_c, do_h, prefix, opts) = \
+    parse_command_line("u", ["unmask-non-abi-names"])
+
+for o, a in opts:
+    if o in ("-u", "--unmask-non-abi-names"):
+        opt_unmask = True
 
 c_comment = '''
 /*
@@ -173,7 +202,7 @@  fdef.write(mcgen('''
                  prefix=prefix))
 
 schema = QAPISchema(input_file)
-gen = QAPISchemaGenIntrospectVisitor()
+gen = QAPISchemaGenIntrospectVisitor(opt_unmask)
 schema.visit(gen)
 fdef.write(gen.defn)
 fdecl.write(gen.decl)