Re: [Qemu-devel] [PATCH v10 04/30] qapi: Share test_init code in test-qmp-input*

[Markus Armbruster]

Eric Blake <address@hidden> 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 <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> 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,