[Qemu-devel] [PATCH v5 09/46] qapi: Use consistent generated code patterns

[Eric Blake]

We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
- Consistently name the error variable 'err'
- Consistently use the labels 'out' and 'out_obj'
- If allocation fails, jump to the right label rather than
indenting everything else

No change in semantics to the generated code.

Signed-off-by: Eric Blake <address@hidden>
---
 scripts/qapi-commands.py |  22 +++++-----
 scripts/qapi-event.py    |  26 +++++------
 scripts/qapi-visit.py    | 111 ++++++++++++++++++++++++-----------------------
 3 files changed, 80 insertions(+), 79 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 810a897..2151120 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -53,14 +53,14 @@ def gen_call(name, arg_type, ret_type):
     push_indent()
     ret = mcgen('''

-%(lhs)sqmp_%(c_name)s(%(args)s&local_err);
+%(lhs)sqmp_%(c_name)s(%(args)s&err);
 ''',
                 c_name=c_name(name), args=argstr, lhs=lhs)
     if ret_type:
-        ret += gen_err_check('local_err')
+        ret += gen_err_check('err')
         ret += mcgen('''

-qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
+qmp_marshal_output_%(c_name)s(retval, ret, &err);
 ''',
                      c_name=ret_type.c_name())
     pop_indent()
@@ -69,7 +69,7 @@ qmp_marshal_output_%(c_name)s(retval, ret, &local_err);

 def gen_marshal_vars(arg_type, ret_type):
     ret = mcgen('''
-    Error *local_err = NULL;
+    Error *err = NULL;
 ''')

     push_indent()
@@ -127,8 +127,8 @@ md = qapi_dealloc_visitor_new();
 v = qapi_dealloc_get_visitor(md);
 ''')
     else:
-        errparg = '&local_err'
-        errarg = 'local_err'
+        errparg = '&err'
+        errarg = 'err'
         ret += mcgen('''
 v = qmp_input_get_visitor(mi);
 ''')
@@ -171,20 +171,20 @@ def gen_marshal_output(ret_type):

 static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject 
**ret_out, Error **errp)
 {
-    Error *local_err = NULL;
+    Error *err = NULL;
     QmpOutputVisitor *mo = qmp_output_visitor_new();
     QapiDeallocVisitor *md;
     Visitor *v;

     v = qmp_output_get_visitor(mo);
-    visit_type_%(c_name)s(v, &ret_in, "unused", &local_err);
-    if (local_err) {
+    visit_type_%(c_name)s(v, &ret_in, "unused", &err);
+    if (err) {
         goto out;
     }
     *ret_out = qmp_output_get_qobject(mo);

 out:
-    error_propagate(errp, local_err);
+    error_propagate(errp, err);
     qmp_output_visitor_cleanup(mo);
     md = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(md);
@@ -227,7 +227,7 @@ def gen_marshal(name, arg_type, ret_type):
 out:
 ''')
     ret += mcgen('''
-    error_propagate(errp, local_err);
+    error_propagate(errp, err);
 ''')
     ret += gen_marshal_input_visit(arg_type, dealloc=True)
     ret += mcgen('''
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index d15fad9..b43bbc2 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -34,7 +34,7 @@ def gen_event_send(name, arg_type):
 %(proto)s
 {
     QDict *qmp;
-    Error *local_err = NULL;
+    Error *err = NULL;
     QMPEventFuncEmit emit;
 ''',
                 proto=gen_event_send_proto(name, arg_type))
@@ -67,9 +67,9 @@ def gen_event_send(name, arg_type):
     g_assert(v);

     /* Fake visit, as if all members are under a structure */
-    visit_start_struct(v, NULL, "", "%(name)s", 0, &local_err);
-    if (local_err) {
-        goto clean;
+    visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
+    if (err) {
+        goto out;
     }

 ''',
@@ -90,9 +90,9 @@ def gen_event_send(name, arg_type):
                 cast = ''

             ret += mcgen('''
-    visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &local_err);
-    if (local_err) {
-        goto clean;
+    visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err);
+    if (err) {
+        goto out;
     }
 ''',
                          cast=cast,
@@ -108,9 +108,9 @@ def gen_event_send(name, arg_type):

         ret += mcgen('''

-    visit_end_struct(v, &local_err);
-    if (local_err) {
-        goto clean;
+    visit_end_struct(v, &err);
+    if (err) {
+        goto out;
     }

     obj = qmp_output_get_qobject(qov);
@@ -120,18 +120,18 @@ def gen_event_send(name, arg_type):
 ''')

     ret += mcgen('''
-    emit(%(c_enum)s, qmp, &local_err);
+    emit(%(c_enum)s, qmp, &err);

 ''',
                  c_enum=c_enum_const(event_enum_name, name))

     if arg_type and arg_type.members:
         ret += mcgen('''
- clean:
+ out:
     qmp_output_visitor_cleanup(qov);
 ''')
     ret += mcgen('''
-    error_propagate(errp, local_err);
+    error_propagate(errp, err);
     QDECREF(qmp);
 }
 ''')
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index d911b20..9c0328d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -24,7 +24,7 @@ def gen_visit_decl(name, scalar=False):
     if not scalar:
         c_type += '*'
     return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error 
**errp);
+void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj, const char *name, Error 
**errp);
 ''',
                  c_name=c_name(name), c_type=c_type)

@@ -39,20 +39,20 @@ def gen_visit_implicit_struct(typ):
         # Need a forward declaration
         ret += mcgen('''

-static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error 
**errp);
+static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error 
**errp);
 ''',
                      c_type=typ.c_name())

     ret += mcgen('''

-static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error 
**errp)
+static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error 
**errp)
 {
     Error *err = NULL;

-    visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
+    visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err);
     if (!err) {
-        visit_type_%(c_type)s_fields(m, obj, &err);
-        visit_end_implicit_struct(m, err ? NULL : &err);
+        visit_type_%(c_type)s_fields(v, obj, &err);
+        visit_end_implicit_struct(v, err ? NULL : &err);
     }
     error_propagate(errp, err);
 }
@@ -71,7 +71,7 @@ def gen_visit_struct_fields(name, base, members):

     ret += mcgen('''

-static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error 
**errp)
+static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error 
**errp)
 {
     Error *err = NULL;

@@ -81,7 +81,7 @@ static void visit_type_%(c_name)s_fields(Visitor *m, 
%(c_name)s **obj, Error **e

     if base:
         ret += mcgen('''
-visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
 if (err) {
     goto out;
 }
@@ -91,14 +91,14 @@ if (err) {
     for memb in members:
         if memb.optional:
             ret += mcgen('''
-visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
+visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err);
 if (!err && (*obj)->has_%(c_name)s) {
 ''',
                          c_name=c_name(memb.name), name=memb.name)
             push_indent()

         ret += mcgen('''
-visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
+visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
 ''',
                      c_type=memb.type.c_name(), c_name=c_name(memb.name),
                      name=memb.name)
@@ -136,16 +136,16 @@ def gen_visit_struct(name, base, members):
     # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
     ret += mcgen('''

-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;

-    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
&err);
+    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
&err);
     if (!err) {
         if (*obj) {
-            visit_type_%(c_name)s_fields(m, obj, &err);
+            visit_type_%(c_name)s_fields(v, obj, &err);
         }
-        visit_end_struct(m, err ? NULL : &err);
+        visit_end_struct(v, err ? NULL : &err);
     }
     error_propagate(errp, err);
 }
@@ -158,24 +158,24 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
 def gen_visit_list(name, element_type):
     return mcgen('''

-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;
     GenericList *i, **prev;

-    visit_start_list(m, name, &err);
+    visit_start_list(v, name, &err);
     if (err) {
         goto out;
     }

     for (prev = (GenericList **)obj;
-         !err && (i = visit_next_list(m, prev, &err)) != NULL;
+         !err && (i = visit_next_list(v, prev, &err)) != NULL;
          prev = &i) {
         %(c_name)s *native_i = (%(c_name)s *)i;
-        visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
+        visit_type_%(c_elt_type)s(v, &native_i->value, NULL, &err);
     }

-    visit_end_list(m, err ? NULL : &err);
+    visit_end_list(v, err ? NULL : &err);
 out:
     error_propagate(errp, err);
 }
@@ -186,9 +186,9 @@ out:
 def gen_visit_enum(name):
     return mcgen('''

-void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, 
Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, 
Error **errp)
 {
-    visit_type_enum(m, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
+    visit_type_enum(v, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
 }
 ''',
                  c_name=c_name(name), name=name)
@@ -197,17 +197,17 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, 
const char *name, Error
 def gen_visit_alternate(name, variants):
     ret = mcgen('''

-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;

-    visit_start_implicit_struct(m, (void**) obj, sizeof(%(c_name)s), &err);
+    visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err);
     if (err) {
         goto out;
     }
-    visit_get_next_type(m, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, 
&err);
+    visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, 
&err);
     if (err) {
-        goto out_end;
+        goto out_obj;
     }
     switch ((*obj)->kind) {
 ''',
@@ -216,7 +216,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
     for var in variants.variants:
         ret += mcgen('''
     case %(case)s:
-        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
+        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err);
         break;
 ''',
                      case=c_enum_const(variants.tag_member.type.name,
@@ -228,8 +228,8 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
     default:
         abort();
     }
-out_end:
-    visit_end_implicit_struct(m, err ? NULL : &err);
+out_obj:
+    visit_end_implicit_struct(v, err ? NULL : &err);
 out:
     error_propagate(errp, err);
 }
@@ -252,24 +252,26 @@ def gen_visit_union(name, base, variants):

     ret += mcgen('''

-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, 
Error **errp)
 {
     Error *err = NULL;

-    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
&err);
+    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
&err);
     if (err) {
         goto out;
     }
-    if (*obj) {
+    if (!*obj) {
+        goto out_obj;
+    }
 ''',
                  c_name=c_name(name), name=name)

     if base:
         ret += mcgen('''
-        visit_type_%(c_name)s_fields(m, obj, &err);
-        if (err) {
-            goto out_obj;
-        }
+    visit_type_%(c_name)s_fields(v, obj, &err);
+    if (err) {
+        goto out_obj;
+    }
 ''',
                      c_name=c_name(name))

@@ -278,14 +280,14 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
         # we pointlessly use a different key for simple unions
         tag_key = 'type'
     ret += mcgen('''
-        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
-        if (err) {
-            goto out_obj;
-        }
-        if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
-            goto out_obj;
-        }
-        switch ((*obj)->%(c_name)s) {
+    visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
+    if (err) {
+        goto out_obj;
+    }
+    if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
+        goto out_obj;
+    }
+    switch ((*obj)->%(c_name)s) {
 ''',
                  c_type=variants.tag_member.type.c_name(),
                  # TODO ugly special case for simple union
@@ -298,36 +300,35 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
         # TODO ugly special case for simple union
         simple_union_type = var.simple_union_type()
         ret += mcgen('''
-        case %(case)s:
+    case %(case)s:
 ''',
                      case=c_enum_const(variants.tag_member.type.name,
                                        var.name))
         if simple_union_type:
             ret += mcgen('''
-            visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);
+        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
 ''',
                          c_type=simple_union_type.c_name(),
                          c_name=c_name(var.name))
         else:
             ret += mcgen('''
-            visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+        visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
 ''',
                          c_type=var.type.c_name(),
                          c_name=c_name(var.name))
         ret += mcgen('''
-            break;
+        break;
 ''')

     ret += mcgen('''
-        default:
-            abort();
-        }
-out_obj:
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_union(m, !!(*obj)->data, &err);
+    default:
+        abort();
     }
-    visit_end_struct(m, err ? NULL : &err);
+out_obj:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_union(v, !!(*obj)->data, &err);
+    visit_end_struct(v, err ? NULL : &err);
 out:
     error_propagate(errp, err);
 }
-- 
2.4.3