Re: [Qemu-devel] [PULL 092/103] qapi: make string output visitor parse int list

[Eric Blake]

On 06/17/2014 11:41 AM, Michael S. Tsirkin wrote:
> From: Hu Tao <address@hidden>
> 
> Signed-off-by: Hu Tao <address@hidden>
> Acked-by: Michael S. Tsirkin <address@hidden>
> Tested-by: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> 
> MST: split up patch
> ---
>  qapi/string-output-visitor.c       | 229 
> +++++++++++++++++++++++++++++++++++--
>  tests/test-string-output-visitor.c |  38 +++++-
>  2 files changed, 255 insertions(+), 12 deletions(-)
> 

> +
> +    l = sov->ranges;
> +    while (l) {
> +        Range *r = l->data;
> +        format_string(sov, r, l->next != NULL, false);
> +        l = l->next;
> +    }
>  
>      if (sov->human) {
> -        out = g_strdup_printf("%lld (%#llx)", (long long) *obj, (long long) 
> *obj);
> -    } else {
> -        out = g_strdup_printf("%lld", (long long) *obj);
> +        l = sov->ranges;
> +        g_string_append(sov->string, " (");
> +        while (l) {
> +            Range *r = l->data;
> +            format_string(sov, r, l->next != NULL, false);

Am I reading this correctly that in human mode, you are creating the string:

16-31 (16-31)

instead of

16-17 (10-1f)

because you forgot to pass 'true' as the human parameter on one of the
two calls to format_string?  Also, this is a worsening of quality; the
old code would produce

16 (0x10)

to make it obvious which number was hex.

> +static void test_visitor_out_intList(TestOutputVisitorData *data,
> +                                     const void *unused)
> +{
> +    int64_t value[] = {0, 1, 9, 10, 16, 15, 14,
> +        3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};

No test of negative numbers?

> +    str = string_output_get_string(data->sov);
> +    g_assert(str != NULL);
> +    g_assert_cmpstr(str, ==,
> +        "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");

Shouldn't you also test the human output?

Probably worth a followup patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature