From 0ec9a5adaf827c21f3a8f9781d3defc8372fff94 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Fri, 23 Feb 2018 14:53:21 -0500
Subject: [PATCH] qapi: Guarantee NULL obj on input visitor callback error

Our existing input visitors were not very consistent on errors in a
function taking 'TYPE **obj'. These are start_struct(),
start_alternate(), type_str(), and type_any(). next_list() is
similar, but can't fail (see commit 08f9541). 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.

Backports commit e58d695e6c3a5cfa0aa2fc91b87ade017ef28b05 from qemu
---
 qemu/qapi/qapi-visit-core.c      | 26 +++++++++++++++++++++++---
 qemu/qapi/qmp-input-visitor.c    |  4 ++++
 qemu/qapi/string-input-visitor.c |  1 +
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/qemu/qapi/qapi-visit-core.c b/qemu/qapi/qapi-visit-core.c
index e2545bde..d405942f 100644
--- a/qemu/qapi/qapi-visit-core.c
+++ b/qemu/qapi/qapi-visit-core.c
@@ -24,7 +24,13 @@
 void visit_start_struct(Visitor *v, const char *name, void **obj,
                         size_t size, Error **errp)
 {
-    v->start_struct(v, name, obj, size, errp);
+    Error *err = NULL;
+
+    v->start_struct(v, name, obj, size, &err);
+    if (obj && v->type == VISITOR_INPUT) {
+        assert(!err != !*obj);
+    }
+    error_propagate(errp, err);
 }
 
 void visit_end_struct(Visitor *v, Error **errp)
@@ -192,7 +198,14 @@ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
 
 void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp)
 {
-    v->type_str(v, name, obj, errp);
+    Error *err = NULL;
+
+    assert(obj);
+    v->type_str(v, name, obj, &err);
+    if (v->type == VISITOR_INPUT) {
+        assert(!err != !*obj);
+    }
+    error_propagate(errp, err);
 }
 
 void visit_type_number(Visitor *v, const char *name, double *obj,
@@ -203,7 +216,14 @@ void visit_type_number(Visitor *v, const char *name, double *obj,
 
 void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp)
 {
-    v->type_any(v, name, obj, errp);
+    Error *err = NULL;
+
+    assert(obj);
+    v->type_any(v, name, obj, &err);
+    if (v->type == VISITOR_INPUT) {
+        assert(!err != !*obj);
+    }
+    error_propagate(errp, err);
 }
 
 static void output_type_enum(Visitor *v, const char *name, int *obj,
diff --git a/qemu/qapi/qmp-input-visitor.c b/qemu/qapi/qmp-input-visitor.c
index b2c166a1..bc600085 100644
--- a/qemu/qapi/qmp-input-visitor.c
+++ b/qemu/qapi/qmp-input-visitor.c
@@ -123,6 +123,9 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
     QObject *qobj = qmp_input_get_object(qiv, name, true);
     Error *err = NULL;
 
+    if (obj) {
+        *obj = NULL;
+    }
     if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "QDict");
@@ -279,6 +282,7 @@ static void qmp_input_type_str(Visitor *v, const char *name, char **obj,
     QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
 
     if (!qstr) {
+        *obj = NULL;
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "string");
         return;
diff --git a/qemu/qapi/string-input-visitor.c b/qemu/qapi/string-input-visitor.c
index ceceef52..f844475f 100644
--- a/qemu/qapi/string-input-visitor.c
+++ b/qemu/qapi/string-input-visitor.c
@@ -272,6 +272,7 @@ static void parse_type_str(Visitor *v, const char *name, char **obj,
     if (siv->string) {
         *obj = g_strdup(siv->string);
     } else {
+        *obj = NULL;
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "string");
     }