Re: [Qemu-devel] [PATCH v2 07/60] check-qjson: Cover escaped characters more thoroughly, part 1

[Eric Blake]

On 08/17/2018 10:05 AM, Markus Armbruster wrote:
> escaped_string() first tests double quoted strings, then repeats a few
> tests with single quotes.  Repeat all of them: store the strings to
> test without quotes, and wrap them in either kind of quote for
> testing.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>   tests/check-qjson.c | 96 +++++++++++++++++++++++++++------------------
>   1 file changed, 57 insertions(+), 39 deletions(-)
>
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index 188f683317..2f1890929d 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -22,55 +22,73 @@
>   #include "qapi/qmp/qstring.h"
>   #include "qemu-common.h"
>   
> +static QString *from_json_str(const char *jstr, Error **errp, bool single)

Unusual to put Error **errp in the middle rather than last. Worth 
swapping 'single' to come before 'errp'?

>          int skip;

Pre-existing, but since you are touching this, is it worth making this bool?

> +    for (i = 0; test_cases[i].json_in; i++) {
> +        for (j = 0; j < 2; j++) {
> +            cstr = from_json_str(test_cases[i].json_in, &error_abort, j);

Only one caller to adjust here (and maybe a couple more later in the 
series).

That's bike-shedding, so either way,
Reviewed-by: Eric Blake <address@hidden>

> +            g_assert_cmpstr(qstring_get_try_str(cstr),
> +                            ==, test_cases[i].utf8_out);
> +            if (test_cases[i].skip == 0) {
> +                jstr = to_json_str(cstr);
> +                g_assert_cmpstr(jstr, ==, test_cases[i].json_in);
> +                g_free(jstr);
> +            }
> +            qobject_unref(cstr);
>           }
> -
> -        qobject_unref(str);
>       }
>   }
>   

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org