[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str |
Date: |
Thu, 08 Nov 2018 15:36:19 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
David Hildenbrand <address@hidden> writes:
> I found some more ugliness, looking at the tests. I am not sure the test
> is correct here.
>
> test_visitor_in_intList():
>
> v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
>
> -> we expect { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20 }, implying that the
> visitor actually does sorting + duplicate elimination. Now I consider
> this is wrong. We are parsing a list of integers, not an ordered set.
>
> What's your take on this?
I think you're right. Visitors are tied to QAPI, and QAPI does *lists*,
not sets. Lists are more general than sets.
I figure what drove development of this code was a need for sets, so
sets got implemented. Review fail.
If the visitor does lists, whatever needs sets can convert the lists to
sets.
I'd advise against evolving the current code towards sanity. Instead,
rewrite, and rely on tests to avoid unwanted changes in behavior.
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, (continued)
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, Eric Blake, 2018/11/05
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, Markus Armbruster, 2018/11/05
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, David Hildenbrand, 2018/11/06
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, Markus Armbruster, 2018/11/07
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, David Hildenbrand, 2018/11/07
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, Markus Armbruster, 2018/11/08
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, David Hildenbrand, 2018/11/08
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, Markus Armbruster, 2018/11/08
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, David Hildenbrand, 2018/11/08
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, David Hildenbrand, 2018/11/08
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, Eric Blake, 2018/11/08
- Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str, David Hildenbrand, 2018/11/08