summary refs log tree commit diff stats
path: root/tests/test-qmp-input-visitor.c
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2016-04-28 15:45:32 -0600
committerMarkus Armbruster <armbru@redhat.com>2016-05-12 09:47:55 +0200
commit68ab47e4b4ecc1c4649362b8cc1e49794d1a6537 (patch)
treec3fb21b91ab1fde06d3471e02f32f694aa18837a /tests/test-qmp-input-visitor.c
parentd9f62dde1303286b24ac8ce88be27e2b9b9c5f46 (diff)
downloadfocaccia-qemu-68ab47e4b4ecc1c4649362b8cc1e49794d1a6537.tar.gz
focaccia-qemu-68ab47e4b4ecc1c4649362b8cc1e49794d1a6537.zip
qapi: Change visit_type_FOO() to no longer return partial objects
Returning a partial object on error is an invitation for a careless
caller to leak memory.  We already fixed things in an earlier
patch to guarantee NULL if visit_start fails ("qapi: Guarantee
NULL obj on input visitor callback error"), but that does not
help the case where visit_start succeeds but some other failure
happens before visit_end, such that we leak a partially constructed
object outside visit_type_FOO(). As no one outside the testsuite
was actually relying on these semantics, it is cleaner to just
document and guarantee that ALL pointer-based visit_type_FOO()
functions always leave a safe value in *obj during an input visitor
(either the new object on success, or NULL if an error is
encountered), so callers can now unconditionally use
qapi_free_FOO() to clean up regardless of whether an error occurred.

The decision is done by adding visit_is_input(), then updating the
generated code to check if additional cleanup is needed based on
the type of visitor in use.

Note that we still leave *obj unchanged after a scalar-based
visit_type_FOO(); I did not feel like auditing all uses of
visit_type_Enum() to see if the callers would tolerate a specific
sentinel value (not to mention having to decide whether it would
be better to use 0 or ENUM__MAX as that sentinel).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-25-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Diffstat (limited to 'tests/test-qmp-input-visitor.c')
-rw-r--r--tests/test-qmp-input-visitor.c10
1 files changed, 2 insertions, 8 deletions
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 6617276bce..cee07ce8dd 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -773,18 +773,12 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
 
     visit_type_TestStruct(v, NULL, &p, &err);
     error_free_or_abort(&err);
-    /* FIXME - a failed parse should not leave a partially-allocated p
-     * for us to clean up; this could cause callers to leak memory. */
-    g_assert(p->string == NULL);
-
-    g_free(p->string);
-    g_free(p);
+    g_assert(!p);
 
     v = visitor_input_test_init(data, "[ '1', '2', false, '3' ]");
     visit_type_strList(v, NULL, &q, &err);
     error_free_or_abort(&err);
-    assert(q);
-    qapi_free_strList(q);
+    assert(!q);
 }
 
 static void test_visitor_in_wrong_type(TestInputVisitorData *data,