On Tue, Jun 17, 2014 at 03:45:29PM -0600, Eric Blake wrote: > On 06/17/2014 11:41 AM, Michael S. Tsirkin wrote: > > From: Hu Tao <hu...@cn.fujitsu.com> > > > > Signed-off-by: Hu Tao <hu...@cn.fujitsu.com> > > Acked-by: Michael S. Tsirkin <m...@redhat.com> > > Tested-by: Michael S. Tsirkin <m...@redhat.com> > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > > > 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.
Hu Tao, could you add tests please? I have fixed up other issues. > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >