diff mbox

[v9,04/27] qapi: Simplify error testing in test-qmp-*

Message ID 1446618049-13596-5-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Nov. 4, 2015, 6:20 a.m. UTC
By using &error_abort, we can avoid a local err variable in
situations where we expect success.

By moving err into data, we can let test teardown take care
of cleaning up any collected error; it also gives us fewer
lines of code between repeated tests where init runs teardown
on our behalf.

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

---
v9: move earlier in series (was 14/17), reword commit message
v8: no change
v7: pick up whitespace changes dropped from earlier commit
v6: new patch
---
 tests/test-qmp-input-strict.c      |  77 +++++++++-----------------
 tests/test-qmp-input-visitor.c     | 107 ++++++++++++-------------------------
 tests/test-qmp-output-visitor.c    |  56 +++++--------------
 tests/test-visitor-serialization.c |  42 ++++++---------
 4 files changed, 89 insertions(+), 193 deletions(-)

Comments

Markus Armbruster Nov. 4, 2015, 8:40 a.m. UTC | #1
Eric Blake <eblake@redhat.com> writes:

> By using &error_abort, we can avoid a local err variable in
> situations where we expect success.

This part is a no-brainer.

Bonus: before we abort(), we print

    Unexpected error in FUNC() at FILE:LINE:
    THE-ERROR-MESSAGE

to stdout on unexpected errors, which is a whole lot more useful than
what g_assert(!err) prints.

By the way: Coccinelle job :)

> By moving err into data, we can let test teardown take care
> of cleaning up any collected error; it also gives us fewer
> lines of code between repeated tests where init runs teardown
> on our behalf.

This part isn't as obvious.

Having two parts of differing obviousness indicates patch splitting
could make sense.  Especially when the parts are large and mechanical,
because reviewing large mechanical changes is much easier when there's
just one kind of it.

> Signed-off-by: Eric Blake <eblake@redhat.com>
[...]
> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
> index 03c3682..d63c470 100644
[...]
> @@ -364,21 +341,17 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
>      /* Parsing an int */
>
>      v = visitor_input_test_init(data, "42");
> -    visit_type_AltStrBool(v, &asb, NULL, &err);
> -    g_assert(err);
> -    error_free(err);
> -    err = NULL;
> +    visit_type_AltStrBool(v, &asb, NULL, &data->err);
> +    g_assert(data->err);
>      qapi_free_AltStrBool(asb);

This leaves data->err non-null.

>
>      /* FIXME: Order of alternate should not affect semantics; asn should
>       * parse the same as ans */
>      v = visitor_input_test_init(data, "42");
> -    visit_type_AltStrNum(v, &asn, NULL, &err);
> +    visit_type_AltStrNum(v, &asn, NULL, &data->err);

If visit_type_AltStrNum() errors out, its error will be thrown away by
its error_propagate(), because data->err is already non-null.

If it fails to error out, its error_propagate() does nothing, and we
won't detect the test failure.

Your patch makes forgetting to reset err an even easier mistake to make,
because it removes most of the error_free() that serve as reminders.

Is it a good idea anyway?  Let's discuss before I check the remainder of
this patch.

>      /* 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;
> +    g_assert(data->err);
>      qapi_free_AltStrNum(asn);
>
>      v = visitor_input_test_init(data, "42");
[...]
Eric Blake Nov. 4, 2015, 9:05 p.m. UTC | #2
On 11/04/2015 01:40 AM, Markus Armbruster wrote:

> 
>> By moving err into data, we can let test teardown take care
>> of cleaning up any collected error; it also gives us fewer
>> lines of code between repeated tests where init runs teardown
>> on our behalf.
> 
> This part isn't as obvious.
> 
> Having two parts of differing obviousness indicates patch splitting
> could make sense.  Especially when the parts are large and mechanical,
> because reviewing large mechanical changes is much easier when there's
> just one kind of it.

Will split.

>> @@ -364,21 +341,17 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
>>      /* Parsing an int */
>>
>>      v = visitor_input_test_init(data, "42");
>> -    visit_type_AltStrBool(v, &asb, NULL, &err);
>> -    g_assert(err);
>> -    error_free(err);
>> -    err = NULL;
>> +    visit_type_AltStrBool(v, &asb, NULL, &data->err);
>> +    g_assert(data->err);
>>      qapi_free_AltStrBool(asb);
> 
> This leaves data->err non-null.
> 
>>
>>      /* FIXME: Order of alternate should not affect semantics; asn should
>>       * parse the same as ans */
>>      v = visitor_input_test_init(data, "42");

And this part wipes out data, so that data->err is once again NULL.

>> -    visit_type_AltStrNum(v, &asn, NULL, &err);
>> +    visit_type_AltStrNum(v, &asn, NULL, &data->err);
> 
> If visit_type_AltStrNum() errors out, its error will be thrown away by
> its error_propagate(), because data->err is already non-null.

No, you missed that this patch is relying on the magic in 3/27 that
wipes out data on every new test.

> 
> If it fails to error out, its error_propagate() does nothing, and we
> won't detect the test failure.
> 
> Your patch makes forgetting to reset err an even easier mistake to make,
> because it removes most of the error_free() that serve as reminders.
> 
> Is it a good idea anyway?  Let's discuss before I check the remainder of
> this patch.

The point was that by moving err _into_ data, then the resetting of err
becomes as simple as resetting data, rather than having to be verbose at
every use of data->err.  We just need a visitor_input_test_init() in
between.
Markus Armbruster Nov. 5, 2015, 7:53 a.m. UTC | #3
Eric Blake <eblake@redhat.com> writes:

> On 11/04/2015 01:40 AM, Markus Armbruster wrote:
>
>> 
>>> By moving err into data, we can let test teardown take care
>>> of cleaning up any collected error; it also gives us fewer
>>> lines of code between repeated tests where init runs teardown
>>> on our behalf.
>> 
>> This part isn't as obvious.
>> 
>> Having two parts of differing obviousness indicates patch splitting
>> could make sense.  Especially when the parts are large and mechanical,
>> because reviewing large mechanical changes is much easier when there's
>> just one kind of it.
>
> Will split.
>
>>> @@ -364,21 +341,17 @@ static void
>>> test_visitor_in_alternate_number(TestInputVisitorData *data,
>>>      /* Parsing an int */
>>>
>>>      v = visitor_input_test_init(data, "42");
>>> -    visit_type_AltStrBool(v, &asb, NULL, &err);
>>> -    g_assert(err);
>>> -    error_free(err);
>>> -    err = NULL;
>>> +    visit_type_AltStrBool(v, &asb, NULL, &data->err);
>>> +    g_assert(data->err);
>>>      qapi_free_AltStrBool(asb);
>> 
>> This leaves data->err non-null.
>> 
>>>
>>>      /* FIXME: Order of alternate should not affect semantics; asn should
>>>       * parse the same as ans */
>>>      v = visitor_input_test_init(data, "42");
>
> And this part wipes out data, so that data->err is once again NULL.
>
>>> -    visit_type_AltStrNum(v, &asn, NULL, &err);
>>> +    visit_type_AltStrNum(v, &asn, NULL, &data->err);
>> 
>> If visit_type_AltStrNum() errors out, its error will be thrown away by
>> its error_propagate(), because data->err is already non-null.
>
> No, you missed that this patch is relying on the magic in 3/27 that
> wipes out data on every new test.

You're right.

>> If it fails to error out, its error_propagate() does nothing, and we
>> won't detect the test failure.
>> 
>> Your patch makes forgetting to reset err an even easier mistake to make,
>> because it removes most of the error_free() that serve as reminders.
>> 
>> Is it a good idea anyway?  Let's discuss before I check the remainder of
>> this patch.
>
> The point was that by moving err _into_ data, then the resetting of err
> becomes as simple as resetting data, rather than having to be verbose at
> every use of data->err.  We just need a visitor_input_test_init() in
> between.

Reducing boilerplate is good.  However, I'm afraid hiding
error_free(err); err = NULL; like this sets an example that is easily
copied incorrectly.

Here's what error.h has to say about consuming an Error object:

 * Report an error to stderr:
 *     error_report_err(err);
 * This frees the error object.
 *
 * Report an error somewhere else:
 *     const char *msg = error_get_pretty(err);
 *     do with msg what needs to be done...
 *     error_free(err);
 *
 * Handle an error without reporting it (just for completeness):
 *     error_free(err);

Perhaps we want something like

 * Expect an error, abort() if there is none:
 *     error_free_or_abort(&err);
 * This frees the error object and clears err.  Convenient for tests.

Then the patch quoted above becomes

    @@ -364,21 +341,17 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
         /* Parsing an int */

         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
          * parse the same as ans */
         v = visitor_input_test_init(data, "42");
         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");

Similar boilerplate reduction, but keeps the clearing of err more
visible.
Eric Blake Nov. 5, 2015, 3:04 p.m. UTC | #4
On 11/05/2015 12:53 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 11/04/2015 01:40 AM, Markus Armbruster wrote:
>>
>>>
>>>> By moving err into data, we can let test teardown take care
>>>> of cleaning up any collected error; it also gives us fewer
>>>> lines of code between repeated tests where init runs teardown
>>>> on our behalf.
>>>
>>> This part isn't as obvious.
>>>
>>> Having two parts of differing obviousness indicates patch splitting
>>> could make sense.  Especially when the parts are large and mechanical,
>>> because reviewing large mechanical changes is much easier when there's
>>> just one kind of it.
>>
>> Will split.

> Perhaps we want something like
> 
>  * Expect an error, abort() if there is none:
>  *     error_free_or_abort(&err);
>  * This frees the error object and clears err.  Convenient for tests.

Don't know if we'd want that directly in error.h, or just in the
affected tests, but I like the idea.  All the more reason for me to
split the patch into expected error vs. expected no error cases.
diff mbox

Patch

diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 910e2f9..f8da75c 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -26,6 +26,7 @@ 
 typedef struct TestInputVisitorData {
     QObject *obj;
     QmpInputVisitor *qiv;
+    Error *err;
 } TestInputVisitorData;

 static void validate_teardown(TestInputVisitorData *data,
@@ -34,6 +35,9 @@  static void validate_teardown(TestInputVisitorData *data,
     qobject_decref(data->obj);
     data->obj = NULL;

+    error_free(data->err);
+    data->err = NULL;
+
     if (data->qiv) {
         qmp_input_visitor_cleanup(data->qiv);
         data->qiv = NULL;
@@ -96,13 +100,11 @@  static void test_validate_struct(TestInputVisitorData *data,
                                   const void *unused)
 {
     TestStruct *p = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");

-    visit_type_TestStruct(v, &p, NULL, &err);
-    g_assert(!err);
+    visit_type_TestStruct(v, &p, NULL, &error_abort);
     g_free(p->string);
     g_free(p);
 }
@@ -111,7 +113,6 @@  static void test_validate_struct_nested(TestInputVisitorData *data,
                                          const void *unused)
 {
     UserDefTwo *udp = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = validate_test_init(data, "{ 'string0': 'string0', "
@@ -119,8 +120,7 @@  static void test_validate_struct_nested(TestInputVisitorData *data,
                            "'dict2': { 'userdef': { 'integer': 42, "
                            "'string': 'string' }, 'string': 'string2'}}}");

-    visit_type_UserDefTwo(v, &udp, NULL, &err);
-    g_assert(!err);
+    visit_type_UserDefTwo(v, &udp, NULL, &error_abort);
     qapi_free_UserDefTwo(udp);
 }

@@ -128,13 +128,11 @@  static void test_validate_list(TestInputVisitorData *data,
                                 const void *unused)
 {
     UserDefOneList *head = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]");

-    visit_type_UserDefOneList(v, &head, NULL, &err);
-    g_assert(!err);
+    visit_type_UserDefOneList(v, &head, NULL, &error_abort);
     qapi_free_UserDefOneList(head);
 }

@@ -143,12 +141,10 @@  static void test_validate_union_native_list(TestInputVisitorData *data,
 {
     UserDefNativeListUnion *tmp = NULL;
     Visitor *v;
-    Error *err = NULL;

     v = validate_test_init(data, "{ 'type': 'integer', 'data' : [ 1, 2 ] }");

-    visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
-    g_assert(!err);
+    visit_type_UserDefNativeListUnion(v, &tmp, NULL, &error_abort);
     qapi_free_UserDefNativeListUnion(tmp);
 }

@@ -157,7 +153,6 @@  static void test_validate_union_flat(TestInputVisitorData *data,
 {
     UserDefFlatUnion *tmp = NULL;
     Visitor *v;
-    Error *err = NULL;

     v = validate_test_init(data,
                            "{ 'enum1': 'value1', "
@@ -165,8 +160,7 @@  static void test_validate_union_flat(TestInputVisitorData *data,
                            "'string': 'str', "
                            "'boolean': true }");

-    visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
-    g_assert(!err);
+    visit_type_UserDefFlatUnion(v, &tmp, NULL, &error_abort);
     qapi_free_UserDefFlatUnion(tmp);
 }

@@ -175,12 +169,10 @@  static void test_validate_alternate(TestInputVisitorData *data,
 {
     UserDefAlternate *tmp = NULL;
     Visitor *v;
-    Error *err = NULL;

     v = validate_test_init(data, "42");

-    visit_type_UserDefAlternate(v, &tmp, NULL, &err);
-    g_assert(!err);
+    visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
     qapi_free_UserDefAlternate(tmp);
 }

@@ -188,14 +180,12 @@  static void test_validate_fail_struct(TestInputVisitorData *data,
                                        const void *unused)
 {
     TestStruct *p = NULL;
-    Error *err = NULL;
     Visitor *v;

     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);
+    visit_type_TestStruct(v, &p, NULL, &data->err);
+    g_assert(data->err);
     /* FIXME: visitor should not allocate p when returning error */
     if (p) {
         g_free(p->string);
@@ -207,14 +197,12 @@  static void test_validate_fail_struct_nested(TestInputVisitorData *data,
                                               const void *unused)
 {
     UserDefTwo *udp = NULL;
-    Error *err = NULL;
     Visitor *v;

     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);
+    visit_type_UserDefTwo(v, &udp, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_UserDefTwo(udp);
 }

@@ -222,14 +210,12 @@  static void test_validate_fail_list(TestInputVisitorData *data,
                                      const void *unused)
 {
     UserDefOneList *head = NULL;
-    Error *err = NULL;
     Visitor *v;

     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);
+    visit_type_UserDefOneList(v, &head, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_UserDefOneList(head);
 }

@@ -237,15 +223,13 @@  static void test_validate_fail_union_native_list(TestInputVisitorData *data,
                                                  const void *unused)
 {
     UserDefNativeListUnion *tmp = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = validate_test_init(data,
                            "{ 'type': 'integer', 'data' : [ 'string' ] }");

-    visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    visit_type_UserDefNativeListUnion(v, &tmp, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_UserDefNativeListUnion(tmp);
 }

@@ -253,14 +237,12 @@  static void test_validate_fail_union_flat(TestInputVisitorData *data,
                                           const void *unused)
 {
     UserDefFlatUnion *tmp = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = validate_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }");

-    visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    visit_type_UserDefFlatUnion(v, &tmp, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_UserDefFlatUnion(tmp);
 }

@@ -268,15 +250,13 @@  static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
                                                      const void *unused)
 {
     UserDefFlatUnion2 *tmp = NULL;
-    Error *err = NULL;
     Visitor *v;

     /* test situation where discriminator field ('enum1' here) is missing */
     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);
+    visit_type_UserDefFlatUnion2(v, &tmp, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_UserDefFlatUnion2(tmp);
 }

@@ -285,13 +265,11 @@  static void test_validate_fail_alternate(TestInputVisitorData *data,
 {
     UserDefAlternate *tmp = NULL;
     Visitor *v;
-    Error *err = NULL;

     v = validate_test_init(data, "3.14");

-    visit_type_UserDefAlternate(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
+    visit_type_UserDefAlternate(v, &tmp, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_UserDefAlternate(tmp);
 }

@@ -299,16 +277,11 @@  static void do_test_validate_qmp_introspect(TestInputVisitorData *data,
                                             const char *schema_json)
 {
     SchemaInfoList *schema = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = validate_test_init_raw(data, schema_json);

-    visit_type_SchemaInfoList(v, &schema, NULL, &err);
-    if (err) {
-        fprintf(stderr, "%s", error_get_pretty(err));
-    }
-    g_assert(!err);
+    visit_type_SchemaInfoList(v, &schema, NULL, &error_abort);
     g_assert(schema);

     qapi_free_SchemaInfoList(schema);
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 03c3682..d63c470 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -22,6 +22,7 @@ 
 typedef struct TestInputVisitorData {
     QObject *obj;
     QmpInputVisitor *qiv;
+    Error *err;
 } TestInputVisitorData;

 static void visitor_input_teardown(TestInputVisitorData *data,
@@ -30,6 +31,9 @@  static void visitor_input_teardown(TestInputVisitorData *data,
     qobject_decref(data->obj);
     data->obj = NULL;

+    error_free(data->err);
+    data->err = NULL;
+
     if (data->qiv) {
         qmp_input_visitor_cleanup(data->qiv);
         data->qiv = NULL;
@@ -92,13 +96,11 @@  static void test_visitor_in_int(TestInputVisitorData *data,
                                 const void *unused)
 {
     int64_t res = 0, value = -42;
-    Error *err = NULL;
     Visitor *v;

     v = visitor_input_test_init(data, "%" PRId64, value);

-    visit_type_int(v, &res, NULL, &err);
-    g_assert(!err);
+    visit_type_int(v, &res, NULL, &error_abort);
     g_assert_cmpint(res, ==, value);
 }

@@ -106,7 +108,6 @@  static void test_visitor_in_int_overflow(TestInputVisitorData *data,
                                          const void *unused)
 {
     int64_t res = 0;
-    Error *err = NULL;
     Visitor *v;

     /* this will overflow a Qint/int64, so should be deserialized into
@@ -115,22 +116,19 @@  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);
+    visit_type_int(v, &res, NULL, &data->err);
+    g_assert(data->err);
 }

 static void test_visitor_in_bool(TestInputVisitorData *data,
                                  const void *unused)
 {
-    Error *err = NULL;
     bool res = false;
     Visitor *v;

     v = visitor_input_test_init(data, "true");

-    visit_type_bool(v, &res, NULL, &err);
-    g_assert(!err);
+    visit_type_bool(v, &res, NULL, &error_abort);
     g_assert_cmpint(res, ==, true);
 }

@@ -138,13 +136,11 @@  static void test_visitor_in_number(TestInputVisitorData *data,
                                    const void *unused)
 {
     double res = 0, value = 3.14;
-    Error *err = NULL;
     Visitor *v;

     v = visitor_input_test_init(data, "%f", value);

-    visit_type_number(v, &res, NULL, &err);
-    g_assert(!err);
+    visit_type_number(v, &res, NULL, &error_abort);
     g_assert_cmpfloat(res, ==, value);
 }

@@ -152,13 +148,11 @@  static void test_visitor_in_string(TestInputVisitorData *data,
                                    const void *unused)
 {
     char *res = NULL, *value = (char *) "Q E M U";
-    Error *err = NULL;
     Visitor *v;

     v = visitor_input_test_init(data, "%s", value);

-    visit_type_str(v, &res, NULL, &err);
-    g_assert(!err);
+    visit_type_str(v, &res, NULL, &error_abort);
     g_assert_cmpstr(res, ==, value);

     g_free(res);
@@ -167,7 +161,6 @@  static void test_visitor_in_string(TestInputVisitorData *data,
 static void test_visitor_in_enum(TestInputVisitorData *data,
                                  const void *unused)
 {
-    Error *err = NULL;
     Visitor *v;
     EnumOne i;

@@ -176,8 +169,7 @@  static void test_visitor_in_enum(TestInputVisitorData *data,

         v = visitor_input_test_init(data, "%s", EnumOne_lookup[i]);

-        visit_type_EnumOne(v, &res, NULL, &err);
-        g_assert(!err);
+        visit_type_EnumOne(v, &res, NULL, &error_abort);
         g_assert_cmpint(i, ==, res);
     }
 }
@@ -187,13 +179,11 @@  static void test_visitor_in_struct(TestInputVisitorData *data,
                                    const void *unused)
 {
     TestStruct *p = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");

-    visit_type_TestStruct(v, &p, NULL, &err);
-    g_assert(!err);
+    visit_type_TestStruct(v, &p, NULL, &error_abort);
     g_assert_cmpint(p->integer, ==, -42);
     g_assert(p->boolean == true);
     g_assert_cmpstr(p->string, ==, "foo");
@@ -206,7 +196,6 @@  static void test_visitor_in_struct_nested(TestInputVisitorData *data,
                                           const void *unused)
 {
     UserDefTwo *udp = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = visitor_input_test_init(data, "{ 'string0': 'string0', "
@@ -214,8 +203,7 @@  static void test_visitor_in_struct_nested(TestInputVisitorData *data,
                                 "'dict2': { 'userdef': { 'integer': 42, "
                                 "'string': 'string' }, 'string': 'string2'}}}");

-    visit_type_UserDefTwo(v, &udp, NULL, &err);
-    g_assert(!err);
+    visit_type_UserDefTwo(v, &udp, NULL, &error_abort);

     g_assert_cmpstr(udp->string0, ==, "string0");
     g_assert_cmpstr(udp->dict1->string1, ==, "string1");
@@ -231,14 +219,12 @@  static void test_visitor_in_list(TestInputVisitorData *data,
                                  const void *unused)
 {
     UserDefOneList *item, *head = NULL;
-    Error *err = NULL;
     Visitor *v;
     int i;

     v = visitor_input_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]");

-    visit_type_UserDefOneList(v, &head, NULL, &err);
-    g_assert(!err);
+    visit_type_UserDefOneList(v, &head, NULL, &error_abort);
     g_assert(head != NULL);

     for (i = 0, item = head; item; item = item->next, i++) {
@@ -256,7 +242,6 @@  static void test_visitor_in_any(TestInputVisitorData *data,
                                 const void *unused)
 {
     QObject *res = NULL;
-    Error *err = NULL;
     Visitor *v;
     QInt *qint;
     QBool *qbool;
@@ -265,16 +250,14 @@  static void test_visitor_in_any(TestInputVisitorData *data,
     QObject *qobj;

     v = visitor_input_test_init(data, "-42");
-    visit_type_any(v, &res, NULL, &err);
-    g_assert(!err);
+    visit_type_any(v, &res, NULL, &error_abort);
     qint = qobject_to_qint(res);
     g_assert(qint);
     g_assert_cmpint(qint_get_int(qint), ==, -42);
     qobject_decref(res);

     v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");
-    visit_type_any(v, &res, NULL, &err);
-    g_assert(!err);
+    visit_type_any(v, &res, NULL, &error_abort);
     qdict = qobject_to_qdict(res);
     g_assert(qdict && qdict_size(qdict) == 3);
     qobj = qdict_get(qdict, "integer");
@@ -299,7 +282,6 @@  static void test_visitor_in_union_flat(TestInputVisitorData *data,
                                        const void *unused)
 {
     Visitor *v;
-    Error *err = NULL;
     UserDefFlatUnion *tmp;
     UserDefUnionBase *base;

@@ -309,8 +291,7 @@  static void test_visitor_in_union_flat(TestInputVisitorData *data,
                                 "'string': 'str', "
                                 "'boolean': true }");

-    visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefFlatUnion(v, &tmp, NULL, &error_abort);
     g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1);
     g_assert_cmpstr(tmp->string, ==, "str");
     g_assert_cmpint(tmp->integer, ==, 41);
@@ -326,7 +307,6 @@  static void test_visitor_in_alternate(TestInputVisitorData *data,
                                       const void *unused)
 {
     Visitor *v;
-    Error *err = NULL;
     UserDefAlternate *tmp;

     v = visitor_input_test_init(data, "42");
@@ -342,10 +322,8 @@  static void test_visitor_in_alternate(TestInputVisitorData *data,
     qapi_free_UserDefAlternate(tmp);

     v = visitor_input_test_init(data, "false");
-    visit_type_UserDefAlternate(v, &tmp, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    visit_type_UserDefAlternate(v, &tmp, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_UserDefAlternate(tmp);
 }

@@ -353,7 +331,6 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,
                                              const void *unused)
 {
     Visitor *v;
-    Error *err = NULL;
     AltStrBool *asb;
     AltStrNum *asn;
     AltNumStr *ans;
@@ -364,21 +341,17 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     /* Parsing an int */

     v = visitor_input_test_init(data, "42");
-    visit_type_AltStrBool(v, &asb, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    visit_type_AltStrBool(v, &asb, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_AltStrBool(asb);

     /* FIXME: Order of alternate should not affect semantics; asn should
      * parse the same as ans */
     v = visitor_input_test_init(data, "42");
-    visit_type_AltStrNum(v, &asn, NULL, &err);
+    visit_type_AltStrNum(v, &asn, NULL, &data->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;
+    g_assert(data->err);
     qapi_free_AltStrNum(asn);

     v = visitor_input_test_init(data, "42");
@@ -408,10 +381,8 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     /* Parsing a double */

     v = visitor_input_test_init(data, "42.5");
-    visit_type_AltStrBool(v, &asb, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    visit_type_AltStrBool(v, &asb, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_AltStrBool(asb);

     v = visitor_input_test_init(data, "42.5");
@@ -427,10 +398,8 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,
     qapi_free_AltNumStr(ans);

     v = visitor_input_test_init(data, "42.5");
-    visit_type_AltStrInt(v, &asi, NULL, &err);
-    g_assert(err);
-    error_free(err);
-    err = NULL;
+    visit_type_AltStrInt(v, &asi, NULL, &data->err);
+    g_assert(data->err);
     qapi_free_AltStrInt(asi);

     v = visitor_input_test_init(data, "42.5");
@@ -451,7 +420,6 @@  static void test_native_list_integer_helper(TestInputVisitorData *data,
                                             UserDefNativeListUnionKind kind)
 {
     UserDefNativeListUnion *cvalue = NULL;
-    Error *err = NULL;
     Visitor *v;
     GString *gstr_list = g_string_new("");
     GString *gstr_union = g_string_new("");
@@ -468,8 +436,7 @@  static void test_native_list_integer_helper(TestInputVisitorData *data,
                            gstr_list->str);
     v = visitor_input_test_init_raw(data,  gstr_union->str);

-    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
     g_assert(cvalue != NULL);
     g_assert_cmpint(cvalue->type, ==, kind);

@@ -614,7 +581,6 @@  static void test_visitor_in_native_list_bool(TestInputVisitorData *data,
 {
     UserDefNativeListUnion *cvalue = NULL;
     boolList *elem = NULL;
-    Error *err = NULL;
     Visitor *v;
     GString *gstr_list = g_string_new("");
     GString *gstr_union = g_string_new("");
@@ -631,8 +597,7 @@  static void test_visitor_in_native_list_bool(TestInputVisitorData *data,
                            gstr_list->str);
     v = visitor_input_test_init_raw(data,  gstr_union->str);

-    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
     g_assert(cvalue != NULL);
     g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);

@@ -650,7 +615,6 @@  static void test_visitor_in_native_list_string(TestInputVisitorData *data,
 {
     UserDefNativeListUnion *cvalue = NULL;
     strList *elem = NULL;
-    Error *err = NULL;
     Visitor *v;
     GString *gstr_list = g_string_new("");
     GString *gstr_union = g_string_new("");
@@ -666,8 +630,7 @@  static void test_visitor_in_native_list_string(TestInputVisitorData *data,
                            gstr_list->str);
     v = visitor_input_test_init_raw(data,  gstr_union->str);

-    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
     g_assert(cvalue != NULL);
     g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);

@@ -689,7 +652,6 @@  static void test_visitor_in_native_list_number(TestInputVisitorData *data,
 {
     UserDefNativeListUnion *cvalue = NULL;
     numberList *elem = NULL;
-    Error *err = NULL;
     Visitor *v;
     GString *gstr_list = g_string_new("");
     GString *gstr_union = g_string_new("");
@@ -705,8 +667,7 @@  static void test_visitor_in_native_list_number(TestInputVisitorData *data,
                            gstr_list->str);
     v = visitor_input_test_init_raw(data,  gstr_union->str);

-    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
     g_assert(cvalue != NULL);
     g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);

@@ -739,18 +700,16 @@  static void test_visitor_in_errors(TestInputVisitorData *data,
                                    const void *unused)
 {
     TestStruct *p = NULL;
-    Error *err = NULL;
     Visitor *v;

     v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }");

-    visit_type_TestStruct(v, &p, NULL, &err);
-    g_assert(err);
+    visit_type_TestStruct(v, &p, NULL, &data->err);
+    g_assert(data->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/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 8606111..0164984 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -45,11 +45,9 @@  static void test_visitor_out_int(TestOutputVisitorData *data,
                                  const void *unused)
 {
     int64_t value = -42;
-    Error *err = NULL;
     QObject *obj;

-    visit_type_int(data->ov, &value, NULL, &err);
-    g_assert(!err);
+    visit_type_int(data->ov, &value, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -62,12 +60,10 @@  static void test_visitor_out_int(TestOutputVisitorData *data,
 static void test_visitor_out_bool(TestOutputVisitorData *data,
                                   const void *unused)
 {
-    Error *err = NULL;
     bool value = true;
     QObject *obj;

-    visit_type_bool(data->ov, &value, NULL, &err);
-    g_assert(!err);
+    visit_type_bool(data->ov, &value, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -81,11 +77,9 @@  static void test_visitor_out_number(TestOutputVisitorData *data,
                                     const void *unused)
 {
     double value = 3.14;
-    Error *err = NULL;
     QObject *obj;

-    visit_type_number(data->ov, &value, NULL, &err);
-    g_assert(!err);
+    visit_type_number(data->ov, &value, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -99,11 +93,9 @@  static void test_visitor_out_string(TestOutputVisitorData *data,
                                     const void *unused)
 {
     char *string = (char *) "Q E M U";
-    Error *err = NULL;
     QObject *obj;

-    visit_type_str(data->ov, &string, NULL, &err);
-    g_assert(!err);
+    visit_type_str(data->ov, &string, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -117,12 +109,10 @@  static void test_visitor_out_no_string(TestOutputVisitorData *data,
                                        const void *unused)
 {
     char *string = NULL;
-    Error *err = NULL;
     QObject *obj;

     /* A null string should return "" */
-    visit_type_str(data->ov, &string, NULL, &err);
-    g_assert(!err);
+    visit_type_str(data->ov, &string, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -135,13 +125,11 @@  static void test_visitor_out_no_string(TestOutputVisitorData *data,
 static void test_visitor_out_enum(TestOutputVisitorData *data,
                                   const void *unused)
 {
-    Error *err = NULL;
     QObject *obj;
     EnumOne i;

     for (i = 0; i < ENUM_ONE_MAX; i++) {
-        visit_type_EnumOne(data->ov, &i, "unused", &err);
-        g_assert(!err);
+        visit_type_EnumOne(data->ov, &i, "unused", &error_abort);

         obj = qmp_output_get_qobject(data->qov);
         g_assert(obj != NULL);
@@ -174,12 +162,10 @@  static void test_visitor_out_struct(TestOutputVisitorData *data,
                                .boolean = false,
                                .string = (char *) "foo"};
     TestStruct *p = &test_struct;
-    Error *err = NULL;
     QObject *obj;
     QDict *qdict;

-    visit_type_TestStruct(data->ov, &p, NULL, &err);
-    g_assert(!err);
+    visit_type_TestStruct(data->ov, &p, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -198,7 +184,6 @@  static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
                                            const void *unused)
 {
     int64_t value = 42;
-    Error *err = NULL;
     UserDefTwo *ud2;
     QObject *obj;
     QDict *qdict, *dict1, *dict2, *dict3, *userdef;
@@ -225,8 +210,7 @@  static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
     ud2->dict1->dict3->userdef->integer = value;
     ud2->dict1->dict3->string = g_strdup(strings[3]);

-    visit_type_UserDefTwo(data->ov, &ud2, "unused", &err);
-    g_assert(!err);
+    visit_type_UserDefTwo(data->ov, &ud2, "unused", &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -288,7 +272,6 @@  static void test_visitor_out_list(TestOutputVisitorData *data,
     const int max_items = 10;
     bool value_bool = true;
     int value_int = 10;
-    Error *err = NULL;
     QListEntry *entry;
     QObject *obj;
     QList *qlist;
@@ -306,8 +289,7 @@  static void test_visitor_out_list(TestOutputVisitorData *data,
         head = p;
     }

-    visit_type_TestStructList(data->ov, &head, NULL, &err);
-    g_assert(!err);
+    visit_type_TestStructList(data->ov, &head, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -367,7 +349,6 @@  static void test_visitor_out_any(TestOutputVisitorData *data,
                                  const void *unused)
 {
     QObject *qobj;
-    Error *err = NULL;
     QInt *qint;
     QBool *qbool;
     QString *qstring;
@@ -375,8 +356,7 @@  static void test_visitor_out_any(TestOutputVisitorData *data,
     QObject *obj;

     qobj = QOBJECT(qint_from_int(-42));
-    visit_type_any(data->ov, &qobj, NULL, &err);
-    g_assert(!err);
+    visit_type_any(data->ov, &qobj, NULL, &error_abort);
     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
     g_assert(qobject_type(obj) == QTYPE_QINT);
@@ -389,8 +369,7 @@  static void test_visitor_out_any(TestOutputVisitorData *data,
     qdict_put(qdict, "boolean", qbool_from_bool(true));
     qdict_put(qdict, "string", qstring_from_str("foo"));
     qobj = QOBJECT(qdict);
-    visit_type_any(data->ov, &qobj, NULL, &err);
-    g_assert(!err);
+    visit_type_any(data->ov, &qobj, NULL, &error_abort);
     qobject_decref(qobj);
     obj = qmp_output_get_qobject(data->qov);
     g_assert(obj != NULL);
@@ -420,8 +399,6 @@  static void test_visitor_out_union_flat(TestOutputVisitorData *data,
     QObject *arg;
     QDict *qdict;

-    Error *err = NULL;
-
     UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
     tmp->enum1 = ENUM_ONE_VALUE1;
     tmp->string = g_strdup("str");
@@ -429,8 +406,7 @@  static void test_visitor_out_union_flat(TestOutputVisitorData *data,
     tmp->integer = 41;
     tmp->u.value1->boolean = true;

-    visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &error_abort);
     arg = qmp_output_get_qobject(data->qov);

     g_assert(qobject_type(arg) == QTYPE_QDICT);
@@ -449,14 +425,12 @@  static void test_visitor_out_alternate(TestOutputVisitorData *data,
                                        const void *unused)
 {
     QObject *arg;
-    Error *err = NULL;

     UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
     tmp->type = USER_DEF_ALTERNATE_KIND_I;
     tmp->u.i = 42;

-    visit_type_UserDefAlternate(data->ov, &tmp, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefAlternate(data->ov, &tmp, NULL, &error_abort);
     arg = qmp_output_get_qobject(data->qov);

     g_assert(qobject_type(arg) == QTYPE_QINT);
@@ -697,14 +671,12 @@  static void test_native_list(TestOutputVisitorData *data,
                              UserDefNativeListUnionKind kind)
 {
     UserDefNativeListUnion *cvalue = g_new0(UserDefNativeListUnion, 1);
-    Error *err = NULL;
     QObject *obj;

     cvalue->type = kind;
     init_native_list(cvalue);

-    visit_type_UserDefNativeListUnion(data->ov, &cvalue, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefNativeListUnion(data->ov, &cvalue, NULL, &error_abort);

     obj = qmp_output_get_qobject(data->qov);
     check_native_list(obj, cvalue->type);
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index c024e5e..9f67f9e 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -302,14 +302,13 @@  static void test_primitives(gconstpointer opaque)
     const SerializeOps *ops = args->ops;
     PrimitiveType *pt = args->test_data;
     PrimitiveType *pt_copy = g_malloc0(sizeof(*pt_copy));
-    Error *err = NULL;
     void *serialize_data;

     pt_copy->type = pt->type;
-    ops->serialize(pt, &serialize_data, visit_primitive_type, &err);
-    ops->deserialize((void **)&pt_copy, serialize_data, visit_primitive_type, &err);
+    ops->serialize(pt, &serialize_data, visit_primitive_type, &error_abort);
+    ops->deserialize((void **)&pt_copy, serialize_data, visit_primitive_type,
+                     &error_abort);

-    g_assert(err == NULL);
     g_assert(pt_copy != NULL);
     if (pt->type == PTYPE_STRING) {
         g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string);
@@ -345,7 +344,6 @@  static void test_primitive_lists(gconstpointer opaque)
     PrimitiveList pl = { .value = { NULL } };
     PrimitiveList pl_copy = { .value = { NULL } };
     PrimitiveList *pl_copy_ptr = &pl_copy;
-    Error *err = NULL;
     void *serialize_data;
     void *cur_head = NULL;
     int i;
@@ -492,10 +490,11 @@  static void test_primitive_lists(gconstpointer opaque)
         }
     }

-    ops->serialize((void **)&pl, &serialize_data, visit_primitive_list, &err);
-    ops->deserialize((void **)&pl_copy_ptr, serialize_data, visit_primitive_list, &err);
+    ops->serialize((void **)&pl, &serialize_data, visit_primitive_list,
+                   &error_abort);
+    ops->deserialize((void **)&pl_copy_ptr, serialize_data,
+                     visit_primitive_list, &error_abort);

-    g_assert(err == NULL);
     i = 0;

     /* compare our deserialized list of primitives to the original */
@@ -652,10 +651,8 @@  static void test_primitive_lists(gconstpointer opaque)
     g_assert_cmpint(i, ==, 33);

     ops->cleanup(serialize_data);
-    dealloc_helper(&pl, visit_primitive_list, &err);
-    g_assert(!err);
-    dealloc_helper(&pl_copy, visit_primitive_list, &err);
-    g_assert(!err);
+    dealloc_helper(&pl, visit_primitive_list, &error_abort);
+    dealloc_helper(&pl_copy, visit_primitive_list, &error_abort);
     g_free(args);
 }

@@ -665,13 +662,12 @@  static void test_struct(gconstpointer opaque)
     const SerializeOps *ops = args->ops;
     TestStruct *ts = struct_create();
     TestStruct *ts_copy = NULL;
-    Error *err = NULL;
     void *serialize_data;

-    ops->serialize(ts, &serialize_data, visit_struct, &err);
-    ops->deserialize((void **)&ts_copy, serialize_data, visit_struct, &err); 
+    ops->serialize(ts, &serialize_data, visit_struct, &error_abort);
+    ops->deserialize((void **)&ts_copy, serialize_data, visit_struct,
+                     &error_abort);

-    g_assert(err == NULL);
     struct_compare(ts, ts_copy);

     struct_cleanup(ts);
@@ -687,14 +683,12 @@  static void test_nested_struct(gconstpointer opaque)
     const SerializeOps *ops = args->ops;
     UserDefTwo *udnp = nested_struct_create();
     UserDefTwo *udnp_copy = NULL;
-    Error *err = NULL;
     void *serialize_data;

-    ops->serialize(udnp, &serialize_data, visit_nested_struct, &err);
+    ops->serialize(udnp, &serialize_data, visit_nested_struct, &error_abort);
     ops->deserialize((void **)&udnp_copy, serialize_data, visit_nested_struct,
-                     &err);
+                     &error_abort);

-    g_assert(err == NULL);
     nested_struct_compare(udnp, udnp_copy);

     nested_struct_cleanup(udnp);
@@ -709,7 +703,6 @@  static void test_nested_struct_list(gconstpointer opaque)
     TestArgs *args = (TestArgs *) opaque;
     const SerializeOps *ops = args->ops;
     UserDefTwoList *listp = NULL, *tmp, *tmp_copy, *listp_copy = NULL;
-    Error *err = NULL;
     void *serialize_data;
     int i = 0;

@@ -720,11 +713,10 @@  static void test_nested_struct_list(gconstpointer opaque)
         listp = tmp;
     }

-    ops->serialize(listp, &serialize_data, visit_nested_struct_list, &err);
+    ops->serialize(listp, &serialize_data, visit_nested_struct_list,
+                   &error_abort);
     ops->deserialize((void **)&listp_copy, serialize_data,
-                     visit_nested_struct_list, &err); 
-
-    g_assert(err == NULL);
+                     visit_nested_struct_list, &error_abort);

     tmp = listp;
     tmp_copy = listp_copy;