qapi: Simplify error cleanup in test-qmp-*

We have several tests that perform multiple sub-actions that are
expected to fail.  Asserting that an error occurred, then clearing
it up to prepare for the next action, turned into enough
boilerplate that it was sometimes forgotten (for example, a number
of tests added to test-qmp-input-visitor.c in d88f5fd leaked err).
Worse, if an error is not reset to NULL, we risk invalidating
later use of that error (passing a non-NULL err into a function
is generally a bad idea).  Encapsulate the boilerplate into a
single helper function error_free_or_abort(), and consistently
use it.

The new function is added into error.c for use everywhere,
although it is anticipated that testsuites will be the main
client.

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

diff --git a/include/qapi/error.h b/include/qapi/error.h
index c69dddbbf270451d0dcc4b0717f18bacfc02ee2b..4d42cdc5fddc60135478602d5e15550566482d7c 100644 (file)
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -30,6 +30,10 @@
  * Handle an error without reporting it (just for completeness):
  *     error_free(err);
  *
+ * Assert that an expected error occurred, but clean it up without
+ * reporting it (primarily useful in testsuites):
+ *     error_free_or_abort(&err);
+ *
  * Pass an existing error to the caller:
  *     error_propagate(errp, err);
  * where Error **errp is a parameter, by convention the last one.
@@ -189,6 +193,11 @@ Error *error_copy(const Error *err);
  */
 void error_free(Error *err);
 
+/*
+ * Convenience function to assert that *@errp is set, then silently free it.
+ */
+void error_free_or_abort(Error **errp);
+
 /*
  * Convenience function to error_report() and free @err.
  */

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index f23d8eaf2a5eae3bf11d11dfad9457907c640642..888fb5ffec4c27278cea43de664b6e6a9d451ea7 100644 (file)
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -225,8 +225,7 @@ static void test_dealloc_partial(void)
     assert(ud2->dict1 == NULL);
 
     /* confirm & release construction error */
-    assert(err != NULL);
-    error_free(err);
+    error_free_or_abort(&err);
 
     /* tear down partial object */
     qapi_free_UserDefTwo(ud2);

diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index d91539c647057cf1f8e3cb50ba8614812570198c..f1c2e3ba67403f9919aaca43587ae06f8db5b909 100644 (file)
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -180,8 +180,7 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
     v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }");
 
     visit_type_TestStruct(v, &p, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
     if (p) {
         g_free(p->string);
     }
@@ -198,8 +197,7 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data,
     v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}");
 
     visit_type_UserDefTwo(v, &udp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
     qapi_free_UserDefTwo(udp);
 }
 
@@ -213,8 +211,7 @@ static void test_validate_fail_list(TestInputVisitorData *data,
     v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]");
 
     visit_type_UserDefOneList(v, &head, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
     qapi_free_UserDefOneList(head);
 }
 
@@ -229,8 +226,7 @@ static void test_validate_fail_union_native_list(TestInputVisitorData *data,
                            "{ 'type': 'integer', 'data' : [ 'string' ] }");
 
     visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
     qapi_free_UserDefNativeListUnion(tmp);
 }
 
@@ -244,8 +240,7 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
     v = validate_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }");
 
     visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
     qapi_free_UserDefFlatUnion(tmp);
 }
 
@@ -260,8 +255,7 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
     v = validate_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }");
 
     visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
     qapi_free_UserDefFlatUnion2(tmp);
 }
 
@@ -275,8 +269,7 @@ static void test_validate_fail_alternate(TestInputVisitorData *data,
     v = validate_test_init(data, "3.14");
 
     visit_type_UserDefAlternate(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
     qapi_free_UserDefAlternate(tmp);
 }
 

diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 630a69fefdd728b72bc741b008f9f51b53d7d75d..ef5284daa24cc45213bfd8431f990f8aea0d58a5 100644 (file)
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -111,8 +111,7 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data,
     v = visitor_input_test_init(data, "%f", DBL_MAX);
 
     visit_type_int(v, &res, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    error_free_or_abort(&err);
 }
 
 static void test_visitor_in_bool(TestInputVisitorData *data,
@@ -319,9 +318,7 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
 
     v = visitor_input_test_init(data, "false");
     visit_type_UserDefAlternate(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    error_free_or_abort(&err);
     qapi_free_UserDefAlternate(tmp);
 }
 
@@ -341,9 +338,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
 
     v = visitor_input_test_init(data, "42");
     visit_type_AltStrBool(v, &asb, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    error_free_or_abort(&err);
     qapi_free_AltStrBool(asb);
 
     /* FIXME: Order of alternate should not affect semantics; asn should
@@ -352,9 +347,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     visit_type_AltStrNum(v, &asn, NULL, &err);
     /* FIXME g_assert_cmpint(asn->type, == ALT_STR_NUM_KIND_N); */
     /* FIXME g_assert_cmpfloat(asn->u.n, ==, 42); */
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    error_free_or_abort(&err);
     qapi_free_AltStrNum(asn);
 
     v = visitor_input_test_init(data, "42");
@@ -385,9 +378,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
 
     v = visitor_input_test_init(data, "42.5");
     visit_type_AltStrBool(v, &asb, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    error_free_or_abort(&err);
     qapi_free_AltStrBool(asb);
 
     v = visitor_input_test_init(data, "42.5");
@@ -404,9 +395,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
 
     v = visitor_input_test_init(data, "42.5");
     visit_type_AltStrInt(v, &asi, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    error_free_or_abort(&err);
     qapi_free_AltStrInt(asi);
 
     v = visitor_input_test_init(data, "42.5");
@@ -713,12 +702,11 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
     v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }");
 
     visit_type_TestStruct(v, &p, NULL, &err);
-    g_assert(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);
 
-    error_free(err);
     g_free(p->string);
     g_free(p);
 }

diff --git a/util/error.c b/util/error.c
index 8b86490ba1ef5e979cc8939e384453c8a85e8b23..80c89a20797527d21089efa293f7dc1ea7957539 100644 (file)
--- a/util/error.c
+++ b/util/error.c
@@ -220,6 +220,13 @@ void error_free(Error *err)
     }
 }
 
+void error_free_or_abort(Error **errp)
+{
+    assert(errp && *errp);
+    error_free(*errp);
+    *errp = NULL;
+}
+
 void error_propagate(Error **dst_errp, Error *local_err)
 {
     if (!local_err) {