[Qemu-devel] [PATCH RFC v3 08/20] qapi: Don't pass pre-existing error to later call

[Eric Blake]

Due to the existing semantics of the error_set() family,
generated sequences in the qapi visitors such as:

    visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err);
        if (!err) {
            visit_type_FOO_fields(m, obj, errp);
            visit_end_implicit_struct(m, &err);
        }
    error_propagate(errp, err);

are risky: if visit_type_FOO_fields() sets errp, and then
visit_end_implicit_struct() also encounters an error, the
attempt to overwrite the first error will cause an abort().
Obviously, we weren't triggering this situation (none of the
existing callbacks for visit_end_implicit_struct() currently
try to set an error), but it is better to not even cause the
problem in the first place.

Meanwhile, in spite of the poor documentation of the qapi
visitors, we want to guarantee that if a visit_start_*()
succeeds, then the matching visit_end_*() will be called.
The options are to either propagate and clear a local err
multiple times:

    visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err);
        if (!err) {
            visit_type_FOO_fields(m, obj, &err);
            if (err) {
                error_propagate(errp, err);
                err = NULL;
            }
            visit_end_implicit_struct(m, &err);
        }
    error_propagate(errp, err);

or, as this patch does, just pass in NULL to ignore further
errors once an error has occurred.

    visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err);
        if (!err) {
            visit_type_FOO_fields(m, obj, &err);
            visit_end_implicit_struct(m, err ? NULL : &err);
        }
    error_propagate(errp, err);

Signed-off-by: Eric Blake <address@hidden>
---
 scripts/qapi-visit.py | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index a3a81c2..cea0414 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -49,8 +49,8 @@ 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, errp);
-        visit_end_implicit_struct(m, &err);
+        visit_type_%(c_type)s_fields(m, obj, &err);
+        visit_end_implicit_struct(m, err ? NULL : &err);
     }
     error_propagate(errp, err);
 }
@@ -142,9 +142,9 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
     visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
&err);
     if (!err) {
         if (*obj) {
-            visit_type_%(c_name)s_fields(m, obj, errp);
+            visit_type_%(c_name)s_fields(m, obj, &err);
         }
-        visit_end_struct(m, &err);
+        visit_end_struct(m, err ? NULL : &err);
     }
     error_propagate(errp, err);
 }
@@ -173,9 +173,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
         visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
     }

-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_list(m, &err);
+    visit_end_list(m, err ? NULL : &err);
 out:
     error_propagate(errp, err);
 }
@@ -232,9 +230,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
                    "%(name)s");
     }
 out_end:
-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_implicit_struct(m, &err);
+    visit_end_implicit_struct(m, err ? NULL : &err);
 out:
     error_propagate(errp, err);
 }
@@ -326,10 +322,8 @@ out_obj:
         error_propagate(errp, err);
         err = NULL;
         visit_end_union(m, !!(*obj)->data, &err);
-        error_propagate(errp, err);
-        err = NULL;
     }
-    visit_end_struct(m, &err);
+    visit_end_struct(m, err ? NULL : &err);
 out:
     error_propagate(errp, err);
 }
-- 
2.4.3