Eric Blake <ebl...@redhat.com> writes:
> By using &error_abort, we can avoid a local err variable in
> situations where we expect success. It also has the nice
> effect that if the test breaks, the error message from
> error_abort tends to be nicer than that of g_assert().
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
[Boring mechanical changes snipped...]
> 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);
This looks like a (very minor) bug fix / cleanup: you're not supposed to
pass the same &err to multiple functions without checking and clearing
it in between, because the second failure trips assert(*errp == NULL) in
error_setv(). Harmless here, but it's nice to get rid of a bad example.
Several more below.
> 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;
Suggest to note the cleanup in the commit message.
