Re: [Qemu-devel] [PATCH v6 09/23] hmp: Improve use of qapi visitor

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]

On 11/25/2015 05:23 PM, Eric Blake wrote:
> Cache the visitor in a local variable instead of repeatedly
> calling the accessor.  Pass NULL for the visit_start_struct()
> object (which matches the fact that we were already passing 0
> for the size argument, because we aren't using the visit to
> allocate a qapi struct).  Pass "object" for the struct name,
> for better error messages. Reflow the logic so that we don't
> have to undo an object_add().
> 
> A later patch will then split the error detection currently
> in visit_struct_end(), at which point we can again hoist the
> object_add() to occur before the label as one of the cleanups
> enabled by that split.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 

> 
> -    object_add(type, id, pdict, opts_get_visitor(ov), &err);
> -
>  out_end:
> -    visit_end_struct(opts_get_visitor(ov), &err_end);
> -    if (!err && err_end) {
> -        qmp_object_del(id, NULL);
> +    visit_end_struct(v, &err_end);
> +    if (!err && !err_end) {
> +        object_add(type, id, pdict, v, &err);
>      }

The attempt to avoid a qmp_object_del() cleanup on error was honorable,
but wrong.  Calling visit_end_struct() prior to passing 'v' to
object_add() means that object_add() is now visiting a different level
of {} than it was pre-patch, which showed up as a testsuite breakage
under 'make check-qtest'.  I'm reworking this patch (and the similar
change to vl.c in 10/23) for v7.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]