qapi: Plug leaks in test-qmp-*

Make valgrind happy with the current state of the tests, so that
it is easier to see if future patches introduce new memory problems
without being drowned in noise.  Many of the leaks were due to
calling a second init without tearing down the data from an earlier
visit.  But since teardown is already idempotent, and we already
register teardown as part of input_visitor_test_add(), it is nicer
to just make init() safe to call multiple times than it is to have
to make all tests call teardown.

Another common leak was forgetting to clean up an error object,
after testing that an error was raised.

Another leak was in test_visitor_in_struct_nested(), failing to
clean the base member of UserDefTwo.  Cleaning that up left
check_and_free_str() as dead code (since using the qapi_free_*
takes care of recursion, and we don't want double frees).

A final leak was in test_visitor_out_any(), which was reassigning
the qobj local variable to a subset of the overall structure
needing freeing; it did not result in a use-after-free, but
was not cleaning up all the qdict.

test-qmp-event and test-qmp-commands were already clean.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 4837b313883c0f952597ddb18f0607058fb4e220..821efe025ce1ed484c842c307308e8d0f6fbb2fe 100644 (file)
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -49,6 +49,8 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data,
 {
     Visitor *v;
 
+    validate_teardown(data, NULL);
+
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
@@ -191,6 +193,7 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
 
     visit_type_TestStruct(v, &p, NULL, &err);
     g_assert(err);
+    error_free(err);
     if (p) {
         g_free(p->string);
     }
@@ -208,6 +211,7 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data,
 
     visit_type_UserDefTwo(v, &udp, NULL, &err);
     g_assert(err);
+    error_free(err);
     qapi_free_UserDefTwo(udp);
 }
 
@@ -222,6 +226,7 @@ static void test_validate_fail_list(TestInputVisitorData *data,
 
     visit_type_UserDefOneList(v, &head, NULL, &err);
     g_assert(err);
+    error_free(err);
     qapi_free_UserDefOneList(head);
 }
 
@@ -237,6 +242,7 @@ static void test_validate_fail_union_native_list(TestInputVisitorData *data,
 
     visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
     g_assert(err);
+    error_free(err);
     qapi_free_UserDefNativeListUnion(tmp);
 }
 
@@ -251,6 +257,7 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
 
     visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
     g_assert(err);
+    error_free(err);
     qapi_free_UserDefFlatUnion(tmp);
 }
 
@@ -266,6 +273,7 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
 
     visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
     g_assert(err);
+    error_free(err);
     qapi_free_UserDefFlatUnion2(tmp);
 }
 
@@ -280,6 +288,7 @@ static void test_validate_fail_alternate(TestInputVisitorData *data,
 
     visit_type_UserDefAlternate(v, &tmp, NULL, &err);
     g_assert(err);
+    error_free(err);
     qapi_free_UserDefAlternate(tmp);
 }
 

diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 8856f3169f4883c79c4efbfe647087d50f3a2559..8d49a9724da493d3bea837869a95c984dda95100 100644 (file)
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -45,6 +45,8 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
 {
     Visitor *v;
 
+    visitor_input_teardown(data, NULL);
+
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
@@ -174,12 +176,7 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
         visit_type_EnumOne(v, &res, NULL, &err);
         g_assert(!err);
         g_assert_cmpint(i, ==, res);
-
-        visitor_input_teardown(data, NULL);
     }
-
-    data->obj = NULL;
-    data->qiv = NULL;
 }
 
 
@@ -202,12 +199,6 @@ static void test_visitor_in_struct(TestInputVisitorData *data,
     g_free(p);
 }
 
-static void check_and_free_str(char *str, const char *cmp)
-{
-    g_assert_cmpstr(str, ==, cmp);
-    g_free(str);
-}
-
 static void test_visitor_in_struct_nested(TestInputVisitorData *data,
                                           const void *unused)
 {
@@ -223,17 +214,14 @@ static void test_visitor_in_struct_nested(TestInputVisitorData *data,
     visit_type_UserDefTwo(v, &udp, NULL, &err);
     g_assert(!err);
 
-    check_and_free_str(udp->string0, "string0");
-    check_and_free_str(udp->dict1->string1, "string1");
+    g_assert_cmpstr(udp->string0, ==, "string0");
+    g_assert_cmpstr(udp->dict1->string1, ==, "string1");
     g_assert_cmpint(udp->dict1->dict2->userdef->integer, ==, 42);
-    check_and_free_str(udp->dict1->dict2->userdef->string, "string");
-    check_and_free_str(udp->dict1->dict2->string, "string2");
+    g_assert_cmpstr(udp->dict1->dict2->userdef->string, ==, "string");
+    g_assert_cmpstr(udp->dict1->dict2->string, ==, "string2");
     g_assert(udp->dict1->has_dict3 == false);
 
-    g_free(udp->dict1->dict2->userdef);
-    g_free(udp->dict1->dict2);
-    g_free(udp->dict1);
-    g_free(udp);
+    qapi_free_UserDefTwo(udp);
 }
 
 static void test_visitor_in_list(TestInputVisitorData *data,
@@ -343,14 +331,12 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
     g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_I);
     g_assert_cmpint(tmp->u.i, ==, 42);
     qapi_free_UserDefAlternate(tmp);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "'string'");
     visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
     g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_S);
     g_assert_cmpstr(tmp->u.s, ==, "string");
     qapi_free_UserDefAlternate(tmp);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "false");
     visit_type_UserDefAlternate(v, &tmp, NULL, &err);
@@ -358,7 +344,6 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
     error_free(err);
     err = NULL;
     qapi_free_UserDefAlternate(tmp);
-    visitor_input_teardown(data, NULL);
 }
 
 static void test_visitor_in_alternate_number(TestInputVisitorData *data,
@@ -381,7 +366,6 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     error_free(err);
     err = NULL;
     qapi_free_AltStrBool(asb);
-    visitor_input_teardown(data, NULL);
 
     /* FIXME: Order of alternate should not affect semantics; asn should
      * parse the same as ans */
@@ -393,35 +377,30 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     error_free(err);
     err = NULL;
     qapi_free_AltStrNum(asn);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42");
     visit_type_AltNumStr(v, &ans, NULL, &error_abort);
     g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
     g_assert_cmpfloat(ans->u.n, ==, 42);
     qapi_free_AltNumStr(ans);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42");
     visit_type_AltStrInt(v, &asi, NULL, &error_abort);
     g_assert_cmpint(asi->type, ==, ALT_STR_INT_KIND_I);
     g_assert_cmpint(asi->u.i, ==, 42);
     qapi_free_AltStrInt(asi);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
     g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_I);
     g_assert_cmpint(ain->u.i, ==, 42);
     qapi_free_AltIntNum(ain);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
     g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_I);
     g_assert_cmpint(ani->u.i, ==, 42);
     qapi_free_AltNumInt(ani);
-    visitor_input_teardown(data, NULL);
 
     /* Parsing a double */
 
@@ -431,21 +410,18 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     error_free(err);
     err = NULL;
     qapi_free_AltStrBool(asb);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42.5");
     visit_type_AltStrNum(v, &asn, NULL, &error_abort);
     g_assert_cmpint(asn->type, ==, ALT_STR_NUM_KIND_N);
     g_assert_cmpfloat(asn->u.n, ==, 42.5);
     qapi_free_AltStrNum(asn);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumStr(v, &ans, NULL, &error_abort);
     g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
     g_assert_cmpfloat(ans->u.n, ==, 42.5);
     qapi_free_AltNumStr(ans);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42.5");
     visit_type_AltStrInt(v, &asi, NULL, &err);
@@ -453,21 +429,18 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     error_free(err);
     err = NULL;
     qapi_free_AltStrInt(asi);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42.5");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
     g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_N);
     g_assert_cmpfloat(ain->u.n, ==, 42.5);
     qapi_free_AltIntNum(ain);
-    visitor_input_teardown(data, NULL);
 
     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
     g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_N);
     g_assert_cmpfloat(ani->u.n, ==, 42.5);
     qapi_free_AltNumInt(ani);
-    visitor_input_teardown(data, NULL);
 }
 
 static void test_native_list_integer_helper(TestInputVisitorData *data,

diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 93648432187dad45543d43ab538416ba4c0d0eee..8606111d45db6522a09ea3f1115c3f410f195979 100644 (file)
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -391,6 +391,7 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
     qobj = QOBJECT(qdict);
     visit_type_any(data->ov, &qobj, NULL, &err);
     g_assert(!err);
+    qobject_decref(qobj);
     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
     qdict = qobject_to_qdict(obj);
@@ -411,7 +412,6 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
     g_assert(qstring);
     g_assert_cmpstr(qstring_get_str(qstring), ==, "foo");
     qobject_decref(obj);
-    qobject_decref(qobj);
 }
 
 static void test_visitor_out_union_flat(TestOutputVisitorData *data,
@@ -463,6 +463,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     g_assert_cmpint(qint_get_int(qobject_to_qint(arg)), ==, 42);
 
     qapi_free_UserDefAlternate(tmp);
+    qobject_decref(arg);
 }
 
 static void test_visitor_out_empty(TestOutputVisitorData *data,