Eric Blake <ebl...@redhat.com> writes:
> Rather than duplicate the body of two functions just to
> decide between qobject_from_jsonv() and qobject_from_json(),
> exploit the fact that qobject_from_jsonv() intentionally
> takes 'va_list *' instead of the more common 'va_list', and
> that qobject_from_json() just calls qobject_from_jsonv(,NULL).
Maximizes the common code. Relies on qobject_from_jsonv() treating a
null ap argument specially. Okay, but I would've left the
initialization of data->obj alone, to avoid the less than obvious null
argument.
Two trivial remarks inline.
> For each file, our two existing init functions then become
> thin wrappers around a new internal function, and future
> updates to initialization don't have to be duplicated.
>
> Suggested-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v10: new patch
> ---
> tests/test-qmp-input-strict.c | 48 ++++++++++++++++++++---------------------
> tests/test-qmp-input-visitor.c | 49
> ++++++++++++++++++++----------------------
> 2 files changed, 46 insertions(+), 51 deletions(-)
>
> diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
> index b44184f..77151de 100644
> --- a/tests/test-qmp-input-strict.c
> +++ b/tests/test-qmp-input-strict.c
> @@ -40,9 +40,27 @@ static void validate_teardown(TestInputVisitorData *data,
> }
> }
>
> -/* This is provided instead of a test setup function so that the JSON
> - string used by the tests are kept in the test functions (and not
> - int main()) */
> +/* The various test_init functions are provided instead of a test setup
> + function so that the JSON string used by the tests are kept in the test
> + functions (and not int main()). */
Since you're touching it: "not in main()".
> +static Visitor *validate_test_init_internal(TestInputVisitorData *data,
> + const char *json_string,
> + va_list *ap)
> +{
> + Visitor *v;
> +
> + data->obj = qobject_from_jsonv(json_string, ap);
> + g_assert(data->obj);
> +
> + data->qiv = qmp_input_visitor_new_strict(data->obj);
> + g_assert(data->qiv);
> +
> + v = qmp_input_get_visitor(data->qiv);
> + g_assert(v);
> +
> + return v;
> +}
> +
> static GCC_FMT_ATTR(2, 3)
> Visitor *validate_test_init(TestInputVisitorData *data,
> const char *json_string, ...)
> @@ -51,17 +69,8 @@ Visitor *validate_test_init(TestInputVisitorData *data,
> va_list ap;
>
> va_start(ap, json_string);
> - data->obj = qobject_from_jsonv(json_string, &ap);
> + v = validate_test_init_internal(data, json_string, &ap);
> va_end(ap);
> -
> - g_assert(data->obj != NULL);
> -
> - data->qiv = qmp_input_visitor_new_strict(data->obj);
> - g_assert(data->qiv != NULL);
> -
> - v = qmp_input_get_visitor(data->qiv);
> - g_assert(v != NULL);
> -
> return v;
> }
>
> @@ -75,18 +84,7 @@ Visitor *validate_test_init(TestInputVisitorData *data,
> static Visitor *validate_test_init_raw(TestInputVisitorData *data,
> const char *json_string)
> {
> - Visitor *v;
> -
> - data->obj = qobject_from_json(json_string);
> - g_assert(data->obj != NULL);
> -
> - data->qiv = qmp_input_visitor_new_strict(data->obj);
> - g_assert(data->qiv != NULL);
> -
> - v = qmp_input_get_visitor(data->qiv);
> - g_assert(v != NULL);
> -
> - return v;
> + return validate_test_init_internal(data, json_string, NULL);
> }
>
>
> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
> index 3f6bc4d..933243d 100644
> --- a/tests/test-qmp-input-visitor.c
> +++ b/tests/test-qmp-input-visitor.c
> @@ -36,9 +36,27 @@ static void visitor_input_teardown(TestInputVisitorData
> *data,
> }
> }
>
> -/* This is provided instead of a test setup function so that the JSON
> - string used by the tests are kept in the test functions (and not
> - int main()) */
> +/* The various test_init functions are provided instead of a test setup
> + function so that the JSON string used by the tests are kept in the test
> + functions (and not int main()). */
Likewise.
> +static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
> + const char *json_string,
> + va_list *ap)
> +{
> + Visitor *v;
> +
> + data->obj = qobject_from_jsonv(json_string, ap);
> + g_assert(data->obj);
> +
> + data->qiv = qmp_input_visitor_new(data->obj);
> + g_assert(data->qiv);
> +
> + v = qmp_input_get_visitor(data->qiv);
> + g_assert(v);
> +
> + return v;
> +}
> +
> static GCC_FMT_ATTR(2, 3)
> Visitor *visitor_input_test_init(TestInputVisitorData *data,
> const char *json_string, ...)
> @@ -47,17 +65,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData
> *data,
> va_list ap;
>
> va_start(ap, json_string);
> - data->obj = qobject_from_jsonv(json_string, &ap);
> + v = visitor_input_test_init_internal(data, json_string, &ap);
> va_end(ap);
> -
> - g_assert(data->obj != NULL);
> -
> - data->qiv = qmp_input_visitor_new(data->obj);
> - g_assert(data->qiv != NULL);
> -
> - v = qmp_input_get_visitor(data->qiv);
> - g_assert(v != NULL);
> -
> return v;
> }
>
> @@ -71,19 +80,7 @@ Visitor *visitor_input_test_init(TestInputVisitorData
> *data,
> static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data,
> const char *json_string)
> {
> - Visitor *v;
> -
> - data->obj = qobject_from_json(json_string);
> -
> - g_assert(data->obj != NULL);
> -
> - data->qiv = qmp_input_visitor_new(data->obj);
> - g_assert(data->qiv != NULL);
> -
> - v = qmp_input_get_visitor(data->qiv);
> - g_assert(v != NULL);
> -
> - return v;
> + return visitor_input_test_init_internal(data, json_string, NULL);
> }
>
> static void test_visitor_in_int(TestInputVisitorData *data,
