Re: [Qemu-devel] [PATCH v2 19/23] migration-test: Clean up string interpolation into QMP, part 3

[Eric Blake <eblake@redhat.com>]

On 07/27/2018 10:13 AM, Markus Armbruster wrote:
> Leaving interpolation into JSON to qmp() is more robust than building
> QMP input manually, as explained in the recent commit "tests: Clean up
> string interpolation into QMP input (simple cases)".
> 
> migration-test.c interpolates strings into JSON in a few places:
> 
> * migrate_set_parameter() interpolates string parameter @value as a
>    JSON number.  Change it to long long.  This requires changing
>    migrate_check_parameter() similarly.
> 
> * migrate_set_capability() interpolates string parameter @value as a
>    JSON boolean.  Change it to bool.
> 
> * deprecated_set_speed() interpolates string parameter @value as a
>    JSON number.  Change it to long long.
> 
> Bonus: gets rid of non-literal format strings.  A step towards
> compile-time format string checking without triggering
> -Wformat-nonliteral.
> 
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> ---
>   tests/migration-test.c | 74 +++++++++++++++++-------------------------
>   1 file changed, 29 insertions(+), 45 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 1c1e56b15b..132c30891d 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -315,31 +315,25 @@ static void cleanup(const char *filename)
>   }
>   
>   static void migrate_check_parameter(QTestState *who, const char *parameter,
> -                                    const char *value)
> +                                    long long value)
>   {
>       QDict *rsp_return;
> -    char *result;
>   
>       rsp_return = wait_command(who,
>                                 "{ 'execute': 'query-migrate-parameters' }");
> -    result = g_strdup_printf("%" PRId64,
> -                             qdict_get_try_int(rsp_return,  parameter, -1));
> -    g_assert_cmpstr(result, ==, value);

The old code allows defaulting to -1 if the value is not present;

> -    g_free(result);
> +    g_assert_cmpint(qdict_get_int(rsp_return,  parameter), ==, value);

the new code requires the value to be found.  Matters if any of the 
callers passed "-1" in the old code, but a search of the file doesn't 
spot any such callers. So I think you're okay.

Also, while touching this line, it would be nice to get rid of the 
double space before parameter.

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

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