On 02/26/2017 03:43 PM, Markus Armbruster wrote: > Fix the design flaw demonstrated in the previous commit: new method > check_list() lets input visitors report that unvisited input remains > for a list, exactly like check_struct() lets them report that > unvisited input remains for a struct or union. > > Implement the method for the qobject input visitor (straightforward), > and the string input visitor (less so, due to the magic list syntax > there). The opts visitor's list magic is even more impenetrable, and > all I can do there today is a stub with a FIXME comment. No worse > than before.
Yeah, I know what you mean (having worked on all three visitors in prior
patches). The opts visitor is just painful.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 2de6377..fe7b988 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -326,6 +326,11 @@ static void prop_get_fdt(Object *obj, Visitor *v, const
> char *name,
> return;
> }
> }
> + visit_check_list(v, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> visit_end_list(v, NULL);
This doesn't look right. You want the visit_end_list() call to occur
unconditionally, even when visit_check_list() fails, so as to free any
resources allocated by the visitor.
> +++ b/tests/test-string-input-visitor.c
> @@ -160,17 +160,9 @@ static void test_visitor_in_intList(TestInputVisitorData
> *data,
> /* Would be simpler if the visitor genuinely supported virtual walks */
> visit_start_list(v, NULL, (GenericList **)&res, sizeof(*res),
> &error_abort);
> - tail = res;
> - visit_type_int64(v, NULL, &tail->value, &error_abort);
> - g_assert_cmpint(tail->value, ==, 0);
> - tail = (int64List *)visit_next_list(v, (GenericList *)tail,
> sizeof(*res));
> - g_assert(tail);
> - visit_type_int64(v, NULL, &tail->value, &error_abort);
> - g_assert_cmpint(tail->value, ==, 2);
> - tail = (int64List *)visit_next_list(v, (GenericList *)tail,
> sizeof(*res));
> - g_assert(tail);
> + visit_check_list(v, &err);
You are still calling visit_check_list() after a partial visit, but why
change from a 2/3 visit to a 0/3 visit?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
