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>

1 files changed, 6 insertions, 7 deletions

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 597fb44fc8..5c3edd753a 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -228,14 +228,13 @@ static void test_dealloc_partial(void)
         QDECREF(ud2_dict);
     }
 
-    /* verify partial success */
-    assert(ud2 != NULL);
-    assert(ud2->string0 != NULL);
-    assert(strcmp(ud2->string0, text) == 0);
-    assert(ud2->dict1 == NULL);
-
-    /* confirm & release construction error */
+    /* verify that visit_type_XXX() cleans up properly on error */
     error_free_or_abort(&err);
+    assert(!ud2);
+
+    /* Manually create a partial object, leaving ud2->dict1 at NULL */
+    ud2 = g_new0(UserDefTwo, 1);
+    ud2->string0 = g_strdup(text);
 
     /* tear down partial object */
     qapi_free_UserDefTwo(ud2);