Re: [Qemu-devel] [PATCH v2 05/54] qapi: generate a literal qobject for introspection

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Replace the generated json string with a literal qobject. The later is
> easier to deal with, at run time as well as compile time: adding #if
> conditionals will be easier than in a json string.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  scripts/qapi-introspect.py         | 91 
> ++++++++++++++++++++++----------------
>  monitor.c                          |  2 +-
>  tests/test-qobject-input-visitor.c | 11 +++--
>  docs/devel/qapi-code-gen.txt       | 29 +++++++-----
>  4 files changed, 80 insertions(+), 53 deletions(-)
>
> diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
> index 032bcea491..1b96926fa7 100644
> --- a/scripts/qapi-introspect.py
> +++ b/scripts/qapi-introspect.py
> @@ -12,72 +12,80 @@
>  from qapi import *
>  
>  
> -# Caveman's json.dumps() replacement (we're stuck at Python 2.4)
> -# TODO try to use json.dumps() once we get unstuck
> -def to_json(obj, level=0):
> +def to_c_string(s):
> +    return '"' + s.replace('\\', r'\\').replace('"', r'\"') + '"'
> +
> +
> +def to_qlit(obj, level=0, first_indent=True):

I still think suppress_first_indent=False would be nicer, but it's not
worth arguing now.

> +
> +    def indent(level):
> +        return level * 4 * ' '
> +
> +    ret = ''
> +    if first_indent:
> +        ret += indent(level)
>      if obj is None:
> -        ret = 'null'
> +        ret += 'QLIT_QNULL'
>      elif isinstance(obj, str):
> -        ret = '"' + obj.replace('"', r'\"') + '"'
> +        ret += 'QLIT_QSTR(' + to_c_string(obj) + ')'
>      elif isinstance(obj, list):
> -        elts = [to_json(elt, level + 1)
> +        elts = [to_qlit(elt, level + 1)
>                  for elt in obj]
> -        ret = '[' + ', '.join(elts) + ']'
> +        elts.append(indent(level + 1) + "{}")
> +        ret += 'QLIT_QLIST(((QLitObject[]) {\n'
> +        ret += ',\n'.join(elts) + '\n'
> +        ret += indent(level) + '}))'
>      elif isinstance(obj, dict):
> -        elts = ['"%s": %s' % (key.replace('"', r'\"'),
> -                              to_json(obj[key], level + 1))
> -                for key in sorted(obj.keys())]
> -        ret = '{' + ', '.join(elts) + '}'
> +        elts = []
> +        for key, value in sorted(obj.iteritems()):
> +            elts.append(indent(level + 1) + '{ %s, %s }' %
> +                        (to_c_string(key), to_qlit(value, level + 1, False)))
> +        elts.append(indent(level + 1) + '{}')
> +        ret += 'QLIT_QDICT(((QLitDictEntry[]) {\n'
> +        ret += ',\n'.join(elts) + '\n'
> +        ret += indent(level) + '}))'
>      else:
>          assert False                # not implemented
> -    if level == 1:
> -        ret = '\n' + ret
>      return ret

Note to self: is there simple a way to get rid of the extra pair of
parenthesis in QLIT_QLIST(( ... )) and QLIT_QDICT(( ... ))?

>  
>  
> -def to_c_string(string):
> -    return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
> -
> -

Let's not move to_c_string() without need.

>  class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
>      def __init__(self, unmask):
>          self._unmask = unmask
>          self.defn = None
>          self.decl = None
>          self._schema = None
> -        self._jsons = None
> +        self._qlits = None
>          self._used_types = None
>          self._name_map = None
>  
>      def visit_begin(self, schema):
>          self._schema = schema
> -        self._jsons = []
> +        self._qlits = []
>          self._used_types = []
>          self._name_map = {}
>  
>      def visit_end(self):
>          # visit the types that are actually used
> -        jsons = self._jsons
> -        self._jsons = []
> +        qlits = self._qlits
> +        self._qlits = []
>          for typ in self._used_types:
>              typ.visit(self)
>          # generate C
>          # TODO can generate awfully long lines
> -        jsons.extend(self._jsons)
> -        name = c_name(prefix, protect=False) + 'qmp_schema_json'
> +        qlits.extend(self._qlits)
> +        name = c_name(prefix, protect=False) + 'qmp_schema_qlit'
>          self.decl = mcgen('''
> -extern const char %(c_name)s[];
> +extern const QLitObject %(c_name)s;
>  ''',
>                            c_name=c_name(name))
> -        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;
> +const QLitObject %(c_name)s = %(c_string)s;
>  ''',
>                            c_name=c_name(name),
> -                          c_string=c_string)
> +                          c_string=to_qlit(qlits))
>          self._schema = None
> -        self._jsons = None
> +        self._qlits = None
>          self._used_types = None
>          self._name_map = None
>  
> @@ -111,12 +119,12 @@ const char %(c_name)s[] = %(c_string)s;
>              return '[' + self._use_type(typ.element_type) + ']'
>          return self._name(typ.name)
>  
> -    def _gen_json(self, name, mtype, obj):
> +    def _gen_qlit(self, name, mtype, obj):
>          if mtype not in ('command', 'event', 'builtin', 'array'):
>              name = self._name(name)
>          obj['name'] = name
>          obj['meta-type'] = mtype
> -        self._jsons.append(obj)
> +        self._qlits.append(obj)
>  
>      def _gen_member(self, member):
>          ret = {'name': member.name, 'type': self._use_type(member.type)}
> @@ -132,24 +140,24 @@ const char %(c_name)s[] = %(c_string)s;
>          return {'case': variant.name, 'type': self._use_type(variant.type)}
>  
>      def visit_builtin_type(self, name, info, json_type):
> -        self._gen_json(name, 'builtin', {'json-type': json_type})
> +        self._gen_qlit(name, 'builtin', {'json-type': json_type})
>  
>      def visit_enum_type(self, name, info, values, prefix):
> -        self._gen_json(name, 'enum', {'values': values})
> +        self._gen_qlit(name, 'enum', {'values': values})
>  
>      def visit_array_type(self, name, info, element_type):
>          element = self._use_type(element_type)
> -        self._gen_json('[' + element + ']', 'array', {'element-type': 
> element})
> +        self._gen_qlit('[' + element + ']', 'array', {'element-type': 
> element})
>  
>      def visit_object_type_flat(self, name, info, members, variants):
>          obj = {'members': [self._gen_member(m) for m in members]}
>          if variants:
>              obj.update(self._gen_variants(variants.tag_member.name,
>                                            variants.variants))
> -        self._gen_json(name, 'object', obj)
> +        self._gen_qlit(name, 'object', obj)
>  
>      def visit_alternate_type(self, name, info, variants):
> -        self._gen_json(name, 'alternate',
> +        self._gen_qlit(name, 'alternate',
>                         {'members': [{'type': self._use_type(m.type)}
>                                      for m in variants.variants]})
>  
> @@ -157,13 +165,13 @@ const char %(c_name)s[] = %(c_string)s;
>                        gen, success_response, boxed):
>          arg_type = arg_type or self._schema.the_empty_object_type
>          ret_type = ret_type or self._schema.the_empty_object_type
> -        self._gen_json(name, 'command',
> +        self._gen_qlit(name, 'command',
>                         {'arg-type': self._use_type(arg_type),
>                          'ret-type': self._use_type(ret_type)})
>  
>      def visit_event(self, name, info, arg_type, boxed):
>          arg_type = arg_type or self._schema.the_empty_object_type
> -        self._gen_json(name, 'event', {'arg-type': self._use_type(arg_type)})
> +        self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)})
>  
>  # Debugging aid: unmask QAPI schema's type names
>  # We normally mask them, because they're not QMP wire ABI
> @@ -205,11 +213,18 @@ h_comment = '''
>  
>  fdef.write(mcgen('''
>  #include "qemu/osdep.h"
> +#include "qapi/qmp/qlit.h"
>  #include "%(prefix)sqmp-introspect.h"
>  
>  ''',
>                   prefix=prefix))
>  
> +fdecl.write(mcgen('''
> +#include "qemu/osdep.h"
> +#include "qapi/qmp/qlit.h"
> +
> +'''))
> +
>  schema = QAPISchema(input_file)
>  gen = QAPISchemaGenIntrospectVisitor(opt_unmask)
>  schema.visit(gen)
> diff --git a/monitor.c b/monitor.c
> index e0f880107f..14c27d4b6f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -953,7 +953,7 @@ EventInfoList *qmp_query_events(Error **errp)
>  static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
>                                   Error **errp)
>  {
> -    *ret_data = qobject_from_json(qmp_schema_json, &error_abort);
> +    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
>  }
>  
>  /*
> diff --git a/tests/test-qobject-input-visitor.c 
> b/tests/test-qobject-input-visitor.c
> index bcf02617dc..bdd00f6bd8 100644
> --- a/tests/test-qobject-input-visitor.c
> +++ b/tests/test-qobject-input-visitor.c
> @@ -1247,24 +1247,27 @@ static void 
> test_visitor_in_fail_alternate(TestInputVisitorData *data,
>  }
>  
>  static void do_test_visitor_in_qmp_introspect(TestInputVisitorData *data,
> -                                              const char *schema_json)
> +                                              const QLitObject *qlit)
>  {
>      SchemaInfoList *schema = NULL;
> +    QObject *obj = qobject_from_qlit(qlit);
>      Visitor *v;
>  
> -    v = visitor_input_test_init_raw(data, schema_json);
> +    v = qobject_input_visitor_new(obj);
>  
>      visit_type_SchemaInfoList(v, NULL, &schema, &error_abort);
>      g_assert(schema);
>  
>      qapi_free_SchemaInfoList(schema);
> +    qobject_decref(obj);
> +    visit_free(v);
>  }
>  
>  static void test_visitor_in_qmp_introspect(TestInputVisitorData *data,
>                                             const void *unused)
>  {
> -    do_test_visitor_in_qmp_introspect(data, test_qmp_schema_json);
> -    do_test_visitor_in_qmp_introspect(data, qmp_schema_json);
> +    do_test_visitor_in_qmp_introspect(data, &test_qmp_schema_qlit);
> +    do_test_visitor_in_qmp_introspect(data, &qmp_schema_qlit);
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 9903ac4c19..b653e86bff 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -1295,18 +1295,27 @@ Example:
>      #ifndef EXAMPLE_QMP_INTROSPECT_H
>      #define EXAMPLE_QMP_INTROSPECT_H
>  
> -    extern const char example_qmp_schema_json[];
> +    extern const QLitObject qmp_schema_qlit;
>  
>      #endif
>      $ cat qapi-generated/example-qmp-introspect.c
>  [Uninteresting stuff omitted...]
>  
> -    const char example_qmp_schema_json[] = "["
> -        "{\"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\"}, 
> {\"default\": null, \"name\": \"string\", \"type\": \"str\"}], \"meta-type\": 
> \"object\", \"name\": \"2\"}, "
> -        "{\"element-type\": \"2\", \"meta-type\": \"array\", \"name\": 
> \"[2]\"}, "
> -        "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": 
> \"int\"}, "
> -        "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": 
> \"str\"}]";
> +    const QLitObject example_qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
> +        QLIT_QDICT(((QLitDictEntry[]) {
> +            { "arg-type", QLIT_QSTR("0") },
> +            { "meta-type", QLIT_QSTR("event") },
> +            { "name", QLIT_QSTR("Event") },
> +            { }
> +        })),
> +        QLIT_QDICT(((QLitDictEntry[]) {
> +            { "members", QLIT_QLIST(((QLitObject[]) {
> +                { }
> +            })) },
> +            { "meta-type", QLIT_QSTR("object") },
> +            { "name", QLIT_QSTR("0") },
> +            { }
> +        })),
> +        ...
> +        { }
> +    }));

Preferably without the to_c_string() move:
Reviewed-by: Markus Armbruster <address@hidden>