[Qemu-devel] [PATCH v15 02A/23] fixup! qapi: Guarantee NULL obj on input visitor callback error

[Eric Blake]

[Updated commit message:]

Our existing input visitors were not very consistent on errors
in a function taking 'TYPE **obj' (that is, start_struct(),
start_alternate(), type_str(), and type_any(). next_list() is
similar, except that since commit 08f9541, it can't fail).
While all of them set '*obj' to allocated storage on success,
it was not obvious whether '*obj' was guaranteed safe on failure,
or whether it was left uninitialized.  But a future patch wants
to guarantee that visit_type_FOO() does not leak a partially-
constructed obj back to the caller; it is easier to implement
this if we can reliably state that input visitors assign '*obj'
regardless of success or failure, and that on failure *obj is
NULL.  Add assertions to enforce consistency in the final
setting of err vs. *obj.

The opts-visitor start_struct() doesn't set an error, but it
also was doing a weird check for 0 size; all callers pass in
non-zero size if obj is non-NULL.

The testsuite has at least one spot where we no longer need
to pre-initialize a variable prior to a visit; valgrind confirms
that the test is still fine with the cleanup.

A later patch will document the design constraint implemented
here.

Signed-off-by: Eric Blake <address@hidden>

---
v15: enhance commit message, hoist assertions from later in series
v14: no change
v13: no change
v12: new patch
---
 qapi/qapi-visit-core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 3a131ce..7ad5ff4 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -27,7 +27,7 @@ void visit_start_struct(Visitor *v, const char *name, void 
**obj,

     v->start_struct(v, name, obj, size, &err);
     if (obj && v->type == VISITOR_INPUT) {
-        assert(err || *obj);
+        assert(!err != !*obj);
     }
     error_propagate(errp, err);
 }
@@ -62,11 +62,11 @@ void visit_start_alternate(Visitor *v, const char *name,
     assert(obj && size >= sizeof(GenericAlternate));
     if (v->start_alternate) {
         v->start_alternate(v, name, obj, size, promote_int, &err);
-        if (v->type == VISITOR_INPUT) {
-            assert(err || *obj);
-        }
-        error_propagate(errp, err);
     }
+    if (v->type == VISITOR_INPUT) {
+        assert(!err != !*obj);
+    }
+    error_propagate(errp, err);
 }

 void visit_end_alternate(Visitor *v)
@@ -205,7 +205,7 @@ void visit_type_str(Visitor *v, const char *name, char 
**obj, Error **errp)
     assert(obj);
     v->type_str(v, name, obj, &err);
     if (v->type == VISITOR_INPUT) {
-        assert(err || *obj);
+        assert(!err != !*obj);
     }
     error_propagate(errp, err);
 }
@@ -223,7 +223,7 @@ void visit_type_any(Visitor *v, const char *name, QObject 
**obj, Error **errp)
     assert(obj);
     v->type_any(v, name, obj, &err);
     if (v->type == VISITOR_INPUT) {
-        assert(err || *obj);
+        assert(!err != !*obj);
     }
     error_propagate(errp, err);
 }
-- 
2.5.5