Eric Blake <ebl...@redhat.com> writes: > 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: qemu-sta...@nongnu.org > Signed-off-by: Eric Blake <ebl...@redhat.com> > Reviewed-by: Michael Roth <mdr...@linux.vnet.ibm.com> > Tested-by: Laurent Vivier <lviv...@redhat.com> > > --- > v2: trivial rebase to comment tweak in patch 1 > --- > 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.
Non-native speaker question: if or whether? > */ > } > > diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c > index 8e0dda5..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); How come unvisited tails are diagnosed late? > @@ -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); This looks wrong. Either you expect an error or not. If you do, error_free_or_abort() seems missing. If you don't, the hunk needs to be dropped. > 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); > - /* BUG: 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