On Wed, Jun 18, 2014 at 06:02:09PM +0300, Michael S. Tsirkin wrote:
> 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.
Sure.
Hu
