[Qemu-devel] [PATCH RFC v5 07/32] qapi: Generate comments to simplify splitting for review

[Markus Armbruster]

The effect of the next few patches on generated files is hard to
review, because stuff gets generated in different order, which renders
diffs of the generated files useless.

To get reviewable diffs, we need to split the generated files into
suitable parts: put every top-level declaration in a file named like
the thing declared, so we can diff top-level declarations regardless
of their order in the generated files.

Since I don't feel like parsing C, simply generate a // comment
identifying the declaration right before each top-level declaration.
This lets us split with a simple shell loop:

    mkdir -p o
    for i in q*-{commands,marshal,event,types,visit}.[ch]
    do
        csplit -n 4 -s "$i" '/^\/\//' '{*}'
        for j in xx*
        do
            read h t <$j
            [ "$h" = "//" ] || t=""
            mv $j o/"$i-${j#xx}-${t/ /-}"
        done
    done

Splits each file F into F-NUMB-ID, where NUMB counts up from 0001, and
ID comes from the // comment.

To check the split's sane, we can run

    for i in q*-{commands,marshal,event,types,visit}.[ch]
    do cat o/$i-* | diff $i -
    done

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
---
 scripts/qapi-commands.py |  7 +++++++
 scripts/qapi-event.py    | 12 ++++++++++++
 scripts/qapi-types.py    | 18 ++++++++++++++++++
 scripts/qapi-visit.py    | 12 ++++++++++++
 scripts/qapi.py          |  5 +++++
 5 files changed, 54 insertions(+)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 12bdc4c..c3afb64 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -24,6 +24,7 @@ def generate_command_decl(name, args, ret_type):
             arglist += "bool has_%s, " % c_name(argname)
         arglist += "%s %s, " % (argtype, c_name(argname))
     return mcgen('''
+// decl qmp_%(name)s
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
 ''',
                  ret_type=c_type(ret_type), name=c_name(name),
@@ -160,6 +161,7 @@ def gen_marshal_output(name, ret_type):
         return ""
 
     ret = mcgen('''
+// def qmp_marshal_output_%(c_name)s
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject 
**ret_out, Error **errp)
 {
     Error *local_err = NULL;
@@ -198,10 +200,12 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
     hdr = gen_marshal_input_decl(name, middle_mode)
 
     ret = mcgen('''
+// def qmp_marshal_input_%(name)s
 %(header)s
 {
     Error *local_err = NULL;
 ''',
+                name=name,
                 header=hdr)
 
     if ret_type:
@@ -252,6 +256,7 @@ qmp_register_command("%(name)s", 
qmp_marshal_input_%(c_name)s, %(opts)s);
                      opts=options)
     pop_indent()
     ret = mcgen('''
+// def qmp_init_marshal
 static void qmp_init_marshal(void)
 {
 ''')
@@ -259,6 +264,7 @@ static void qmp_init_marshal(void)
     ret += mcgen('''
 }
 
+// decl qapi_init
 qapi_init(qmp_init_marshal);
 ''')
     return ret
@@ -347,6 +353,7 @@ for cmd in commands:
         fdef.write(ret)
 
     if middle_mode:
+        fdecl.write('// decl gen_marshal_input_%s\n' % cmd['command'])
         fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], 
middle_mode))
 
     ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + 
"\n"
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index aec2d32..e511437 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -9,6 +9,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
+import re;
 from ordereddict import OrderedDict
 from qapi import *
 
@@ -33,22 +34,30 @@ def _generate_event_api_name(event_name, params):
 # Following are the core functions that generate C APIs to emit event.
 
 def generate_event_declaration(api_name):
+    match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
+    name = match.group(1)
     return mcgen('''
 
+// decl %(name)s
 %(api_name)s;
 ''',
+                 name=name,
                  api_name = api_name)
 
 def generate_event_implement(api_name, event_name, params):
+    match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
+    name = match.group(1)
     # step 1: declare any variables
     ret = mcgen("""
 
+// def %(name)s
 %(api_name)s
 {
     QDict *qmp;
     Error *local_err = NULL;
     QMPEventFuncEmit emit;
 """,
+                name=name,
                 api_name = api_name)
 
     if params:
@@ -162,11 +171,13 @@ def generate_event_implement(api_name, event_name, 
params):
 def generate_event_enum_decl(event_enum_name, event_enum_values):
     lookup_decl = mcgen('''
 
+// decl %(event_enum_name)s_lookup
 extern const char *%(event_enum_name)s_lookup[];
 ''',
                         event_enum_name = event_enum_name)
 
     enum_decl = mcgen('''
+// typedef %(event_enum_name)s
 typedef enum %(event_enum_name)s {
 ''',
                       event_enum_name = event_enum_name)
@@ -194,6 +205,7 @@ typedef enum %(event_enum_name)s {
 def generate_event_enum_lookup(event_enum_name, event_enum_strings):
     ret = mcgen('''
 
+// def %(event_enum_name)s_lookup
 const char *%(event_enum_name)s_lookup[] = {
 ''',
                 event_enum_name = event_enum_name)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 124a788..7fabcaa 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -15,8 +15,10 @@ from qapi import *
 def generate_fwd_builtin(name):
     return mcgen('''
 
+// typedef %(name)sList
 typedef struct %(name)sList %(name)sList;
 
+// struct %(name)sList
 struct %(name)sList {
     union {
         %(type)s value;
@@ -31,10 +33,13 @@ struct %(name)sList {
 def generate_fwd_struct(name):
     return mcgen('''
 
+// typedef %(name)s
 typedef struct %(name)s %(name)s;
 
+// typedef %(name)sList
 typedef struct %(name)sList %(name)sList;
 
+// struct %(name)sList
 struct %(name)sList {
     union {
         %(name)s *value;
@@ -48,8 +53,10 @@ struct %(name)sList {
 def generate_fwd_enum_struct(name):
     return mcgen('''
 
+// typedef %(name)sList
 typedef struct %(name)sList %(name)sList;
 
+// struct %(name)sList
 struct %(name)sList {
     union {
         %(name)s value;
@@ -84,6 +91,7 @@ def generate_struct(expr):
 
     ret = mcgen('''
 
+// struct %(name)s
 struct %(name)s {
 ''',
           name=c_name(structname))
@@ -110,6 +118,7 @@ struct %(name)s {
 def generate_enum_lookup(name, values):
     ret = mcgen('''
 
+// def %(name)s_lookup
 const char *const %(name)s_lookup[] = {
 ''',
                 name=c_name(name))
@@ -132,12 +141,14 @@ def generate_enum(name, values):
     name = c_name(name)
     lookup_decl = mcgen('''
 
+// decl %(name)s_lookup
 extern const char *const %(name)s_lookup[];
 ''',
                 name=name)
 
     enum_decl = mcgen('''
 
+// typedef %(name)s
 typedef enum %(name)s {
 ''',
                 name=name)
@@ -169,6 +180,7 @@ def generate_alternate_qtypes(expr):
 
     ret = mcgen('''
 
+// def %(name)s_qtypes
 const int %(name)s_qtypes[QTYPE_MAX] = {
 ''',
                 name=c_name(name))
@@ -205,6 +217,7 @@ def generate_union(expr, meta):
 
     ret = mcgen('''
 
+// struct %(name)s
 struct %(name)s {
 ''',
                 name=name)
@@ -252,6 +265,8 @@ struct %(name)s {
 ''')
     if meta == 'alternate':
         ret += mcgen('''
+
+// decl %(name)s_qtypes
 extern const int %(name)s_qtypes[];
 ''',
             name=name)
@@ -261,6 +276,8 @@ extern const int %(name)s_qtypes[];
 
 def generate_type_cleanup_decl(name):
     ret = mcgen('''
+
+// decl qapi_free_%(name)s
 void qapi_free_%(name)s(%(c_type)s obj);
 ''',
                 c_type=c_type(name), name=c_name(name))
@@ -269,6 +286,7 @@ void qapi_free_%(name)s(%(c_type)s obj);
 def generate_type_cleanup(name):
     ret = mcgen('''
 
+// def qapi_free_%(name)s
 void qapi_free_%(name)s(%(c_type)s obj)
 {
     QapiDeallocVisitor *md;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c493964..f0fef55 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -28,12 +28,14 @@ def generate_visit_implicit_struct(type):
         # Need a forward declaration
         ret += mcgen('''
 
+// def visit_type_%(c_type)s_fields
 static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error 
**errp);
 ''',
                      c_type=type_name(type))
 
     ret += mcgen('''
 
+// def visit_type_implicit_%(c_type)s
 static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error 
**errp)
 {
     Error *err = NULL;
@@ -59,6 +61,7 @@ def generate_visit_struct_fields(name, members, base = None):
 
     ret += mcgen('''
 
+// def visit_type_%(name)s_fields
 static void visit_type_%(name)s_fields(Visitor *m, %(name)s **obj, Error 
**errp)
 {
     Error *err = NULL;
@@ -146,6 +149,7 @@ def generate_visit_struct(expr):
 
     ret += mcgen('''
 
+// def visit_type_%(name)s
 void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error 
**errp)
 {
 ''',
@@ -161,6 +165,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const 
char *name, Error **e
 def generate_visit_list(name):
     return mcgen('''
 
+// def visit_type_%(name)sList
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;
@@ -190,6 +195,7 @@ out:
 def generate_visit_enum(name):
     return mcgen('''
 
+// def visit_type_%(c_name)s
 void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, 
Error **errp)
 {
     visit_type_enum(m, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
@@ -200,6 +206,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, 
const char *name, Error
 def generate_visit_alternate(name, members):
     ret = mcgen('''
 
+// def visit_type_%(name)s
 void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error 
**errp)
 {
     Error *err = NULL;
@@ -283,6 +290,7 @@ def generate_visit_union(expr):
 
     ret += mcgen('''
 
+// def visit_type_%(c_name)s
 void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;
@@ -365,11 +373,13 @@ def generate_declaration(name, builtin_type=False):
         name = c_name(name)
         ret += mcgen('''
 
+// decl visit_type_%(name)s
 void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error 
**errp);
 ''',
                      name=name)
 
     ret += mcgen('''
+// decl visit_type_%(name)sList
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, 
Error **errp);
 ''',
                  name=name)
@@ -378,6 +388,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList 
**obj, const char *name, E
 
 def generate_enum_declaration(name):
     ret = mcgen('''
+// decl visit_type_%(name)sList
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, 
Error **errp);
 ''',
                 name=c_name(name))
@@ -387,6 +398,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList 
**obj, const char *name, E
 def generate_decl_enum(name):
     return mcgen('''
 
+// decl visit_type_%(name)s
 void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error 
**errp);
 ''',
                  name=c_name(name))
diff --git a/scripts/qapi.py b/scripts/qapi.py
index fd52a9a..1a1a4e6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1463,6 +1463,7 @@ def guardname(filename):
 def guardstart(name):
     return mcgen('''
 
+// guardstart %(name)s
 #ifndef %(name)s
 #define %(name)s
 
@@ -1472,6 +1473,7 @@ def guardstart(name):
 def guardend(name):
     return mcgen('''
 
+// guardend %(name)s
 #endif /* %(name)s */
 
 ''',
@@ -1556,12 +1558,14 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, 
h_file,
     fdecl = maybe_open(do_h, h_file, 'w')
 
     fdef.write(mcgen('''
+// prologue
 /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
 %(comment)s
 ''',
                      comment = c_comment))
 
     fdecl.write(mcgen('''
+// prologue
 /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
 %(comment)s
 #ifndef %(guard)s
@@ -1574,6 +1578,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, 
h_file,
 
 def close_output(fdef, fdecl):
     fdecl.write('''
+// epilogue
 #endif
 ''')
     fdecl.close()
-- 
2.4.3