From 68ab47e4b4ecc1c4649362b8cc1e49794d1a6537 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 28 Apr 2016 15:45:32 -0600 Subject: 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 Message-Id: <1461879932-9020-25-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- tests/test-qmp-commands.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'tests/test-qmp-commands.c') diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 597fb44..5c3edd7 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); -- cgit v1.1