[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 092/103] qapi: make string output visitor parse i
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PULL 092/103] qapi: make string output visitor parse int list |
Date: |
Wed, 18 Jun 2014 18:02:09 +0300 |
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 <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.
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
>
- [Qemu-devel] [PULL 077/103] memory: move mem_path handling to memory_region_allocate_system_memory, (continued)
- [Qemu-devel] [PULL 077/103] memory: move mem_path handling to memory_region_allocate_system_memory, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 084/103] hostmem: add merge and dump properties, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 085/103] hostmem: allow preallocation of any memory region, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 089/103] hmp: add info memdev, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 090/103] tests: fix memory leak in test of string input visitor, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 091/103] qapi: make string input visitor parse int list, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 092/103] qapi: make string output visitor parse int list, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 093/103] qapi: fix build on glib < 2.28, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 094/103] qdev: reorganize error reporting in bus_set_realized, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 095/103] qdev: recursively unrealize devices when unrealizing bus, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 096/103] qmp: clean out whitespace, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 048/103] Add G_IO_HUP handler for socket chardev, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 063/103] NUMA: check if the total numa memory size is equal to ram_size, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 064/103] NUMA: Add numa_info structure to contain numa nodes info, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 099/103] qmp: add query-memory-devices command, Michael S. Tsirkin, 2014/06/17
- [Qemu-devel] [PULL 100/103] acpi: introduce TYPE_ACPI_DEVICE_IF interface, Michael S. Tsirkin, 2014/06/17