summary refs log tree commit diff stats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--docs/qapi-code-gen.txt87
-rw-r--r--hw/timer/mc146818rtc.c24
-rw-r--r--hw/virtio/virtio-balloon.c12
-rw-r--r--qapi/qapi-visit-core.c231
-rw-r--r--scripts/qapi-commands.py57
-rw-r--r--scripts/qapi-visit.py171
-rw-r--r--tests/test-qmp-input-strict.c10
-rw-r--r--tests/test-qmp-input-visitor.c26
-rw-r--r--tests/test-qmp-output-visitor.c10
-rw-r--r--tests/test-visitor-serialization.c12
10 files changed, 354 insertions, 286 deletions
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 1a635e2572..1948946c05 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -275,7 +275,6 @@ Example:
         qapi_dealloc_visitor_cleanup(md);
     }
 
-
     void qapi_free_UserDefOne(UserDefOne * obj)
     {
         QapiDeallocVisitor *md;
@@ -352,49 +351,54 @@ Example:
     {
         Error *err = NULL;
         visit_type_int(m, &(*obj)->integer, "integer", &err);
+        if (err) {
+            goto out;
+        }
         visit_type_str(m, &(*obj)->string, "string", &err);
+        if (err) {
+            goto out;
+        }
 
+    out:
         error_propagate(errp, err);
     }
 
     void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char *name, Error **errp)
     {
-        if (!error_is_set(errp)) {
-            Error *err = NULL;
-            visit_start_struct(m, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
-            if (!err) {
-                if (*obj) {
-                    visit_type_UserDefOne_fields(m, obj, &err);
-                    error_propagate(errp, err);
-                    err = NULL;
-                }
-                /* Always call end_struct if start_struct succeeded.  */
-                visit_end_struct(m, &err);
+        Error *err = NULL;
+
+        visit_start_struct(m, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
+        if (!err) {
+            if (*obj) {
+                visit_type_UserDefOne_fields(m, obj, errp);
             }
-            error_propagate(errp, err);
+            visit_end_struct(m, &err);
         }
+        error_propagate(errp, err);
     }
 
     void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp)
     {
-        GenericList *i, **prev = (GenericList **)obj;
         Error *err = NULL;
+        GenericList *i, **prev;
 
-        if (!error_is_set(errp)) {
-            visit_start_list(m, name, &err);
-            if (!err) {
-                for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
-                    UserDefOneList *native_i = (UserDefOneList *)i;
-                    visit_type_UserDefOne(m, &native_i->value, NULL, &err);
-                }
-                error_propagate(errp, err);
-                err = NULL;
-
-                /* Always call end_list if start_list succeeded.  */
-                visit_end_list(m, &err);
-            }
-            error_propagate(errp, err);
+        visit_start_list(m, name, &err);
+        if (err) {
+            goto out;
+        }
+
+        for (prev = (GenericList **)obj;
+             !err && (i = visit_next_list(m, prev, &err)) != NULL;
+             prev = &i) {
+            UserDefOneList *native_i = (UserDefOneList *)i;
+            visit_type_UserDefOne(m, &native_i->value, NULL, &err);
         }
+
+        error_propagate(errp, err);
+        err = NULL;
+        visit_end_list(m, &err);
+    out:
+        error_propagate(errp, err);
     }
     mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.h
 [Uninteresting stuff omitted...]
@@ -434,15 +438,20 @@ Example:
 
     static void qmp_marshal_output_my_command(UserDefOne * ret_in, QObject **ret_out, Error **errp)
     {
+        Error *local_err = NULL;
         QmpOutputVisitor *mo = qmp_output_visitor_new();
         QapiDeallocVisitor *md;
         Visitor *v;
 
         v = qmp_output_get_visitor(mo);
-        visit_type_UserDefOne(v, &ret_in, "unused", errp);
-        if (!error_is_set(errp)) {
-            *ret_out = qmp_output_get_qobject(mo);
+        visit_type_UserDefOne(v, &ret_in, "unused", &local_err);
+        if (local_err) {
+            goto out;
         }
+        *ret_out = qmp_output_get_qobject(mo);
+
+    out:
+        error_propagate(errp, local_err);
         qmp_output_visitor_cleanup(mo);
         md = qapi_dealloc_visitor_new();
         v = qapi_dealloc_get_visitor(md);
@@ -452,6 +461,7 @@ Example:
 
     static void qmp_marshal_input_my_command(QDict *args, QObject **ret, Error **errp)
     {
+        Error *local_err = NULL;
         UserDefOne * retval = NULL;
         QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
         QapiDeallocVisitor *md;
@@ -459,17 +469,20 @@ Example:
         UserDefOne * arg1 = NULL;
 
         v = qmp_input_get_visitor(mi);
-        visit_type_UserDefOne(v, &arg1, "arg1", errp);
-
-        if (error_is_set(errp)) {
+        visit_type_UserDefOne(v, &arg1, "arg1", &local_err);
+        if (local_err) {
             goto out;
         }
-        retval = qmp_my_command(arg1, errp);
-        if (!error_is_set(errp)) {
-            qmp_marshal_output_my_command(retval, ret, errp);
+
+        retval = qmp_my_command(arg1, &local_err);
+        if (local_err) {
+            goto out;
         }
 
+        qmp_marshal_output_my_command(retval, ret, &local_err);
+
     out:
+        error_propagate(errp, local_err);
         qmp_input_visitor_cleanup(mi);
         md = qapi_dealloc_visitor_new();
         v = qapi_dealloc_get_visitor(md);
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 6c3e3b6d75..df54546562 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -804,13 +804,33 @@ static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
         goto out;
     }
     visit_type_int32(v, &current_tm.tm_year, "tm_year", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_int32(v, &current_tm.tm_mon, "tm_mon", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_int32(v, &current_tm.tm_mday, "tm_mday", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_int32(v, &current_tm.tm_hour, "tm_hour", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_int32(v, &current_tm.tm_min, "tm_min", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_int32(v, &current_tm.tm_sec, "tm_sec", &err);
-    visit_end_struct(v, &err);
-
+    if (err) {
+        goto out_end;
+    }
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, errp);
 out:
     error_propagate(errp, err);
 }
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index ca99bd5f97..bf2b588b24 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -121,23 +121,27 @@ static void balloon_stats_get_all(Object *obj, struct Visitor *v,
     if (err) {
         goto out;
     }
-
     visit_type_int(v, &s->stats_last_update, "last-update", &err);
+    if (err) {
+        goto out_end;
+    }
 
     visit_start_struct(v, NULL, NULL, "stats", 0, &err);
     if (err) {
         goto out_end;
     }
-        
-    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
+    for (i = 0; !err && i < VIRTIO_BALLOON_S_NR; i++) {
         visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
                          &err);
     }
+    error_propagate(errp, err);
+    err = NULL;
     visit_end_struct(v, &err);
 
 out_end:
+    error_propagate(errp, err);
+    err = NULL;
     visit_end_struct(v, &err);
-
 out:
     error_propagate(errp, err);
 }
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index ffd76372d1..55f8d4068c 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -20,28 +20,24 @@
 void visit_start_struct(Visitor *v, void **obj, const char *kind,
                         const char *name, size_t size, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->start_struct(v, obj, kind, name, size, errp);
-    }
+    v->start_struct(v, obj, kind, name, size, errp);
 }
 
 void visit_end_struct(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
     v->end_struct(v, errp);
 }
 
 void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
                                  Error **errp)
 {
-    if (!error_is_set(errp) && v->start_implicit_struct) {
+    if (v->start_implicit_struct) {
         v->start_implicit_struct(v, obj, size, errp);
     }
 }
 
 void visit_end_implicit_struct(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
     if (v->end_implicit_struct) {
         v->end_implicit_struct(v, errp);
     }
@@ -49,30 +45,23 @@ void visit_end_implicit_struct(Visitor *v, Error **errp)
 
 void visit_start_list(Visitor *v, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->start_list(v, name, errp);
-    }
+    v->start_list(v, name, errp);
 }
 
 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        return v->next_list(v, list, errp);
-    }
-
-    return 0;
+    return v->next_list(v, list, errp);
 }
 
 void visit_end_list(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
     v->end_list(v, errp);
 }
 
 void visit_optional(Visitor *v, bool *present, const char *name,
                     Error **errp)
 {
-    if (!error_is_set(errp) && v->optional) {
+    if (v->optional) {
         v->optional(v, present, name, errp);
     }
 }
@@ -80,7 +69,7 @@ void visit_optional(Visitor *v, bool *present, const char *name,
 void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
                          const char *name, Error **errp)
 {
-    if (!error_is_set(errp) && v->get_next_type) {
+    if (v->get_next_type) {
         v->get_next_type(v, obj, qtypes, name, errp);
     }
 }
@@ -88,192 +77,172 @@ void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
 void visit_type_enum(Visitor *v, int *obj, const char *strings[],
                      const char *kind, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_enum(v, obj, strings, kind, name, errp);
-    }
+    v->type_enum(v, obj, strings, kind, name, errp);
 }
 
 void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_int(v, obj, name, errp);
-    }
+    v->type_int(v, obj, name, errp);
 }
 
 void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint8) {
-            v->type_uint8(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT8_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint8_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint8) {
+        v->type_uint8(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT8_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint8_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint16) {
-            v->type_uint16(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT16_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint16_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint16) {
+        v->type_uint16(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT16_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint16_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint32) {
-            v->type_uint32(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT32_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint32_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint32) {
+        v->type_uint32(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT32_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint32_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint64) {
-            v->type_uint64(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            *obj = value;
-        }
+
+    if (v->type_uint64) {
+        v->type_uint64(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        *obj = value;
     }
 }
 
 void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int8) {
-            v->type_int8(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT8_MIN || value > INT8_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int8_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int8) {
+        v->type_int8(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT8_MIN || value > INT8_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int8_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int16) {
-            v->type_int16(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT16_MIN || value > INT16_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int16_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int16) {
+        v->type_int16(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT16_MIN || value > INT16_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int16_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int32) {
-            v->type_int32(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT32_MIN || value > INT32_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int32_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int32) {
+        v->type_int32(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT32_MIN || value > INT32_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int32_t");
+            return;
         }
+        *obj = value;
     }
 }
 
 void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        if (v->type_int64) {
-            v->type_int64(v, obj, name, errp);
-        } else {
-            v->type_int(v, obj, name, errp);
-        }
+    if (v->type_int64) {
+        v->type_int64(v, obj, name, errp);
+    } else {
+        v->type_int(v, obj, name, errp);
     }
 }
 
 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 {
     int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_size) {
-            v->type_size(v, obj, name, errp);
-        } else if (v->type_uint64) {
-            v->type_uint64(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            *obj = value;
-        }
+
+    if (v->type_size) {
+        v->type_size(v, obj, name, errp);
+    } else if (v->type_uint64) {
+        v->type_uint64(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        *obj = value;
     }
 }
 
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_bool(v, obj, name, errp);
-    }
+    v->type_bool(v, obj, name, errp);
 }
 
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_str(v, obj, name, errp);
-    }
+    v->type_str(v, obj, name, errp);
 }
 
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_number(v, obj, name, errp);
-    }
+    v->type_number(v, obj, name, errp);
 }
 
 void output_type_enum(Visitor *v, int *obj, const char *strings[],
@@ -299,13 +268,15 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
                      const char *kind, const char *name,
                      Error **errp)
 {
+    Error *local_err = NULL;
     int64_t value = 0;
     char *enum_str;
 
     assert(strings);
 
-    visit_type_str(v, &enum_str, name, errp);
-    if (error_is_set(errp)) {
+    visit_type_str(v, &enum_str, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return;
     }
 
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 341dba27a8..386f17ef44 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -2,16 +2,19 @@
 # QAPI command marshaller generator
 #
 # Copyright IBM, Corp. 2011
+# Copyright (C) 2014 Red Hat, Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
 #  Michael Roth    <mdroth@linux.vnet.ibm.com>
+#  Markus Armbruster <armbru@redhat.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
 from ordereddict import OrderedDict
 from qapi import *
+import re
 import sys
 import os
 import getopt
@@ -37,6 +40,15 @@ def generate_command_decl(name, args, ret_type):
 ''',
                  ret_type=c_type(ret_type), name=c_fun(name), args=arglist).strip()
 
+def gen_err_check(errvar):
+    if errvar:
+        return mcgen('''
+if (local_err) {
+    goto out;
+}
+''')
+    return ''
+
 def gen_sync_call(name, args, ret_type, indent=0):
     ret = ""
     arglist=""
@@ -49,15 +61,14 @@ def gen_sync_call(name, args, ret_type, indent=0):
         arglist += "%s, " % (c_var(argname))
     push_indent(indent)
     ret = mcgen('''
-%(retval)sqmp_%(name)s(%(args)serrp);
+%(retval)sqmp_%(name)s(%(args)s&local_err);
 
 ''',
                 name=c_fun(name), args=arglist, retval=retval).rstrip()
     if ret_type:
+        ret += "\n" + gen_err_check('local_err')
         ret += "\n" + mcgen(''''
-if (!error_is_set(errp)) {
-    %(marshal_output_call)s
-}
+%(marshal_output_call)s
 ''',
                             marshal_output_call=gen_marshal_output_call(name, ret_type)).rstrip()
     pop_indent(indent)
@@ -67,7 +78,7 @@ if (!error_is_set(errp)) {
 def gen_marshal_output_call(name, ret_type):
     if not ret_type:
         return ""
-    return "qmp_marshal_output_%s(retval, ret, errp);" % c_fun(name)
+    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_fun(name)
 
 def gen_visitor_input_containers_decl(args, obj):
     ret = ""
@@ -109,7 +120,8 @@ bool has_%(argname)s = false;
 
 def gen_visitor_input_block(args, dealloc=False):
     ret = ""
-    errparg = 'errp'
+    errparg = '&local_err'
+    errarg = 'local_err'
 
     if len(args) == 0:
         return ret
@@ -118,6 +130,7 @@ def gen_visitor_input_block(args, dealloc=False):
 
     if dealloc:
         errparg = 'NULL'
+        errarg = None;
         ret += mcgen('''
 qmp_input_visitor_cleanup(mi);
 md = qapi_dealloc_visitor_new();
@@ -132,15 +145,20 @@ v = qmp_input_get_visitor(mi);
         if optional:
             ret += mcgen('''
 visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
-if (has_%(c_name)s) {
 ''',
                          c_name=c_var(argname), name=argname, errp=errparg)
+            ret += gen_err_check(errarg)
+            ret += mcgen('''
+if (has_%(c_name)s) {
+''',
+                         c_name=c_var(argname))
             push_indent()
         ret += mcgen('''
 %(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
                      c_name=c_var(argname), name=argname, argtype=argtype,
                      visitor=type_visitor(argtype), errp=errparg)
+        ret += gen_err_check(errarg)
         if optional:
             pop_indent()
             ret += mcgen('''
@@ -161,15 +179,20 @@ def gen_marshal_output(name, args, ret_type, middle_mode):
     ret = mcgen('''
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_out, Error **errp)
 {
+    Error *local_err = NULL;
     QmpOutputVisitor *mo = qmp_output_visitor_new();
     QapiDeallocVisitor *md;
     Visitor *v;
 
     v = qmp_output_get_visitor(mo);
-    %(visitor)s(v, &ret_in, "unused", errp);
-    if (!error_is_set(errp)) {
-        *ret_out = qmp_output_get_qobject(mo);
+    %(visitor)s(v, &ret_in, "unused", &local_err);
+    if (local_err) {
+        goto out;
     }
+    *ret_out = qmp_output_get_qobject(mo);
+
+out:
+    error_propagate(errp, local_err);
     qmp_output_visitor_cleanup(mo);
     md = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(md);
@@ -196,13 +219,12 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
     ret = mcgen('''
 %(header)s
 {
+    Error *local_err = NULL;
 ''',
                 header=hdr)
 
     if middle_mode:
         ret += mcgen('''
-    Error *local_err = NULL;
-    Error **errp = &local_err;
     QDict *args = (QDict *)qdict;
 ''')
 
@@ -229,20 +251,23 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
                      visitor_input_block=gen_visitor_input_block(args))
     else:
         ret += mcgen('''
+
     (void)args;
 ''')
 
     ret += mcgen('''
-    if (error_is_set(errp)) {
-        goto out;
-    }
 %(sync_call)s
 ''',
                  sync_call=gen_sync_call(name, args, ret_type, indent=4))
-    ret += mcgen('''
+    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+        ret += mcgen('''
 
 out:
 ''')
+    if not middle_mode:
+        ret += mcgen('''
+    error_propagate(errp, local_err);
+''')
     ret += mcgen('''
 %(visitor_input_block_cleanup)s
 ''',
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 1ef753a9b9..06a79f1631 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -2,16 +2,19 @@
 # QAPI visitor generator
 #
 # Copyright IBM, Corp. 2011
+# Copyright (C) 2014 Red Hat, Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
 #  Michael Roth    <mdroth@linux.vnet.ibm.com>
+#  Markus Armbruster <armbru@redhat.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
 from ordereddict import OrderedDict
 from qapi import *
+import re
 import sys
 import os
 import getopt
@@ -32,9 +35,7 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error *
 
     visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
     if (!err) {
-        visit_type_%(c_type)s_fields(m, obj, &err);
-        error_propagate(errp, err);
-        err = NULL;
+        visit_type_%(c_type)s_fields(m, obj, errp);
         visit_end_implicit_struct(m, &err);
     }
     error_propagate(errp, err);
@@ -64,12 +65,9 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base =
 
 static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj, Error **errp)
 {
-    Error *err = NULL;
 ''',
                          name=name, full_name=full_name, c_name=c_var(argname))
-            push_indent()
             ret += generate_visit_struct_body(full_name, argname, argentry)
-            pop_indent()
             ret += mcgen('''
 }
 ''')
@@ -89,6 +87,9 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error *
     if base:
         ret += mcgen('''
 visit_type_implicit_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
+if (err) {
+    goto out;
+}
 ''',
                      c_prefix=c_var(field_prefix),
                      type=type_name(base), c_name=c_var('base'))
@@ -97,7 +98,7 @@ visit_type_implicit_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
         if optional:
             ret += mcgen('''
 visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
-if ((*obj)->%(prefix)shas_%(c_name)s) {
+if (!err && (*obj)->%(prefix)shas_%(c_name)s) {
 ''',
                          c_prefix=c_var(field_prefix), prefix=field_prefix,
                          c_name=c_var(argname), name=argname)
@@ -121,10 +122,19 @@ visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
             ret += mcgen('''
 }
 ''')
+        ret += mcgen('''
+if (err) {
+    goto out;
+}
+''')
 
     pop_indent()
-    ret += mcgen('''
+    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+        ret += mcgen('''
 
+out:
+''')
+    ret += mcgen('''
     error_propagate(errp, err);
 }
 ''')
@@ -133,9 +143,9 @@ visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
 
 def generate_visit_struct_body(field_prefix, name, members):
     ret = mcgen('''
-if (!error_is_set(errp)) {
+    Error *err = NULL;
+
 ''')
-    push_indent()
 
     if not field_prefix:
         full_name = name
@@ -144,36 +154,26 @@ if (!error_is_set(errp)) {
 
     if len(field_prefix):
         ret += mcgen('''
-visit_start_struct(m, NULL, "", "%(name)s", 0, &err);
+    visit_start_struct(m, NULL, "", "%(name)s", 0, &err);
 ''',
                 name=name)
     else:
         ret += mcgen('''
-Error *err = NULL;
-visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
 ''',
                 name=name)
 
     ret += mcgen('''
-if (!err) {
-    if (*obj) {
-        visit_type_%(name)s_fields(m, obj, &err);
-        error_propagate(errp, err);
-        err = NULL;
+    if (!err) {
+        if (*obj) {
+            visit_type_%(name)s_fields(m, obj, errp);
+        }
+        visit_end_struct(m, &err);
     }
+    error_propagate(errp, err);
 ''',
         name=full_name)
 
-    ret += mcgen('''
-    /* Always call end_struct if start_struct succeeded.  */
-    visit_end_struct(m, &err);
-}
-error_propagate(errp, err);
-''')
-    pop_indent()
-    ret += mcgen('''
-}
-''')
     return ret
 
 def generate_visit_struct(expr):
@@ -191,9 +191,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 ''',
                 name=name)
 
-    push_indent()
     ret += generate_visit_struct_body("", name, members)
-    pop_indent()
 
     ret += mcgen('''
 }
@@ -205,24 +203,26 @@ def generate_visit_list(name, members):
 
 void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp)
 {
-    GenericList *i, **prev = (GenericList **)obj;
     Error *err = NULL;
+    GenericList *i, **prev;
 
-    if (!error_is_set(errp)) {
-        visit_start_list(m, name, &err);
-        if (!err) {
-            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
-                %(name)sList *native_i = (%(name)sList *)i;
-                visit_type_%(name)s(m, &native_i->value, NULL, &err);
-            }
-            error_propagate(errp, err);
-            err = NULL;
-
-            /* Always call end_list if start_list succeeded.  */
-            visit_end_list(m, &err);
-        }
-        error_propagate(errp, err);
+    visit_start_list(m, name, &err);
+    if (err) {
+        goto out;
+    }
+
+    for (prev = (GenericList **)obj;
+         !err && (i = visit_next_list(m, prev, &err)) != NULL;
+         prev = &i) {
+        %(name)sList *native_i = (%(name)sList *)i;
+        visit_type_%(name)s(m, &native_i->value, NULL, &err);
     }
+
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_list(m, &err);
+out:
+    error_propagate(errp, err);
 }
 ''',
                 name=name)
@@ -244,10 +244,15 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 {
     Error *err = NULL;
 
-    if (!error_is_set(errp)) {
-        visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err);
-        visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err);
-        switch ((*obj)->kind) {
+    visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err);
+    if (err) {
+        goto out;
+    }
+    visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err);
+    if (err) {
+        goto out_end;
+    }
+    switch ((*obj)->kind) {
 ''',
     name=name)
 
@@ -262,22 +267,24 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 
         enum_full_value = generate_enum_full_value(disc_type, key)
         ret += mcgen('''
-        case %(enum_full_value)s:
-            visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
-            break;
+    case %(enum_full_value)s:
+        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
+        break;
 ''',
                 enum_full_value = enum_full_value,
                 c_type = type_name(members[key]),
                 c_name = c_fun(key))
 
     ret += mcgen('''
-        default:
-            abort();
-        }
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_implicit_struct(m, &err);
+    default:
+        abort();
     }
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_implicit_struct(m, &err);
+out:
+    error_propagate(errp, err);
 }
 ''')
 
@@ -324,19 +331,20 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 {
     Error *err = NULL;
 
-    if (!error_is_set(errp)) {
-        visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
-        if (!err) {
-            if (*obj) {
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    if (err) {
+        goto out;
+    }
+    if (*obj) {
 ''',
                  name=name)
 
-    push_indent()
-    push_indent()
-
     if base:
         ret += mcgen('''
         visit_type_%(name)s_fields(m, obj, &err);
+        if (err) {
+            goto out_obj;
+        }
 ''',
             name=name)
 
@@ -346,8 +354,10 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
         disc_key = discriminator
     ret += mcgen('''
         visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
-        if (!err) {
-            switch ((*obj)->kind) {
+        if (err) {
+            goto out_obj;
+        }
+        switch ((*obj)->kind) {
 ''',
                  disc_type = disc_type,
                  disc_key = disc_key)
@@ -360,32 +370,25 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 
         enum_full_value = generate_enum_full_value(disc_type, key)
         ret += mcgen('''
-            case %(enum_full_value)s:
-                ''' + fmt + '''
-                break;
+        case %(enum_full_value)s:
+            ''' + fmt + '''
+            break;
 ''',
                 enum_full_value = enum_full_value,
                 c_type=type_name(members[key]),
                 c_name=c_fun(key))
 
     ret += mcgen('''
-            default:
-                abort();
-            }
+        default:
+            abort();
         }
+out_obj:
         error_propagate(errp, err);
         err = NULL;
-''')
-    pop_indent()
-    pop_indent()
-
-    ret += mcgen('''
-            }
-            /* Always call end_struct if start_struct succeeded.  */
-            visit_end_struct(m, &err);
-        }
-        error_propagate(errp, err);
     }
+    visit_end_struct(m, &err);
+out:
+    error_propagate(errp, err);
 }
 ''')
 
@@ -505,7 +508,7 @@ fdecl.write(mcgen('''
 /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
 
 /*
- * schema-defined QAPI visitor function
+ * schema-defined QAPI visitor functions
  *
  * Copyright IBM, Corp. 2011
  *
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index ec798c2acf..0f770034b1 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -81,11 +81,19 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
     }
 
     visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_str(v, &(*obj)->string, "string", &err);
 
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
     visit_end_struct(v, &err);
-
 out:
     error_propagate(errp, err);
 }
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index a58a3e6fdb..1c8e87295c 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -199,16 +199,24 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
 
     visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
                        &err);
-    if (!err) {
-        visit_type_int(v, &(*obj)->integer, "integer", &err);
-        visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
-        visit_type_str(v, &(*obj)->string, "string", &err);
-
-        /* Always call end_struct if start_struct succeeded.  */
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_struct(v, &err);
+    if (err) {
+        goto out;
     }
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
     error_propagate(errp, err);
 }
 
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index dfd597cee1..9c154581d7 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -185,11 +185,19 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
     }
 
     visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_str(v, &(*obj)->string, "string", &err);
 
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
     visit_end_struct(v, &err);
-
 out:
     error_propagate(errp, err);
 }
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 85170e5c49..74d6481992 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -195,7 +195,7 @@ typedef struct TestStruct
 static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
                                   const char *name, Error **errp)
 {
-    Error *err= NULL;
+    Error *err = NULL;
 
     visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), &err);
     if (err) {
@@ -203,11 +203,19 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
     }
 
     visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
     visit_type_str(v, &(*obj)->string, "string", &err);
 
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
     visit_end_struct(v, &err);
-
 out:
     error_propagate(errp, err);
 }