[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] qapi: Fix QemuOpts visitor regression on un
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] qapi: Fix QemuOpts visitor regression on unvisited input |
Date: |
Tue, 21 Mar 2017 09:19:52 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 21/03/2017 04:17, Eric Blake wrote:
> An off-by-one in commit 15c2f669e meant that we were failing to
> check for unparsed input in all QemuOpts visitors. Recent testsuite
> additions show that fixing the obvious bug with bogus fields will
> also fix the case of an incomplete list visit; update the tests to
> match the new behavior.
>
> Simple testcase:
>
> ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa
> node,size=1g
>
> failed to diagnose that 'size' is not a valid argument to -numa, and
> now once again reports:
>
> qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size'
>
> CC: address@hidden
> Signed-off-by: Eric Blake <address@hidden>
Tested-by: Laurent Vivier <address@hidden>
> ---
> qapi/opts-visitor.c | 6 +++---
> tests/test-opts-visitor.c | 15 +++++++++------
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 026d25b..b54da81 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -164,7 +164,7 @@ opts_check_struct(Visitor *v, Error **errp)
> GHashTableIter iter;
> GQueue *any;
>
> - if (ov->depth > 0) {
> + if (ov->depth > 1) {
> return;
> }
>
> @@ -276,8 +276,8 @@ static void
> opts_check_list(Visitor *v, Error **errp)
> {
> /*
> - * FIXME should set error when unvisited elements remain. Mostly
> - * harmless, as the generated visits always visit all elements.
> + * Unvisited list elements will be reported later when checking if
> + * unvisited struct members remain.
> */
> }
>
> diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
> index a47c344..1766919 100644
> --- a/tests/test-opts-visitor.c
> +++ b/tests/test-opts-visitor.c
> @@ -175,6 +175,7 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer
> test_data)
> static void
> test_opts_range_unvisited(void)
> {
> + Error *err = NULL;
> intList *list = NULL;
> intList *tail;
> QemuOpts *opts;
> @@ -199,10 +200,11 @@ test_opts_range_unvisited(void)
> g_assert_cmpint(tail->value, ==, 1);
> tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list));
> g_assert(tail);
> - visit_check_list(v, &error_abort); /* BUG: unvisited tail not reported */
> + visit_check_list(v, &error_abort); /* unvisited tail ignored until... */
> visit_end_list(v, (void **)&list);
>
> - visit_check_struct(v, &error_abort);
> + visit_check_struct(v, &err); /* ...here */
> + error_free_or_abort(&err);
> visit_end_struct(v, NULL);
>
> qapi_free_intList(list);
> @@ -239,7 +241,7 @@ test_opts_range_beyond(void)
> error_free_or_abort(&err);
> visit_end_list(v, (void **)&list);
>
> - visit_check_struct(v, &error_abort);
> + visit_check_struct(v, &err);
> visit_end_struct(v, NULL);
>
> qapi_free_intList(list);
> @@ -250,6 +252,7 @@ test_opts_range_beyond(void)
> static void
> test_opts_dict_unvisited(void)
> {
> + Error *err = NULL;
> QemuOpts *opts;
> Visitor *v;
> UserDefOptions *userdef;
> @@ -258,11 +261,11 @@ test_opts_dict_unvisited(void)
> &error_abort);
>
> v = opts_visitor_new(opts);
> - /* FIXME: bogus should be diagnosed */
> - visit_type_UserDefOptions(v, NULL, &userdef, &error_abort);
> + visit_type_UserDefOptions(v, NULL, &userdef, &err);
> + error_free_or_abort(&err);
> visit_free(v);
> qemu_opts_del(opts);
> - qapi_free_UserDefOptions(userdef);
> + g_assert(!userdef);
> }
>
> int
>