On 12/03/2015 09:37 AM, Markus Armbruster wrote:
> prop_get_fdt() misuses the visitor API: when fdt is null, it doesn't
> visit anything.  object_property_get_qobject() happily
> object_property_get_qobject().  Amazingly, the latter survives the

Something got lost or otherwise corrupted in that sentence.  Were you
trying to say one function happily calls another?  If so, which of the
two "object_property_get_qobject()" strings should be changed, to what?

> misuse.  Turns out we've papered over it long before prop_get_fdt()
> existed, in commit 1d10b44.
> 
> However, commit 6c2f9a1 changed how we paper over it, and as a side
> effect changed qom-get's value from {} to null.  Change it right back
> by fixing the visitor misuse.
> 
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
>  hw/ppc/spapr_drc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 4e7a1d3..dad157f 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -259,6 +259,11 @@ static void prop_get_fdt(Object *obj, Visitor *v, void 
> *opaque,
>      void *fdt;
>  
>      if (!drc->fdt) {
> +        visit_start_struct(v, NULL, NULL, name, 0, &err);

visit_start_struct(v, NULL, "fdt", name, 0, &err)

would give a nicer error message, but only if there were an error
message to give.  As already mentioned on 1/3, we are only using this
for output visitors which don't raise errors (in fact, &error_abort
would do, rather than worrying about error propagation).

But I'm fine with the patch as you proposed it, once the commit message
is fixed.

Reviewed-by: Eric Blake <ebl...@redhat.com>

> +        if (!err) {
> +            visit_end_struct(v, &err);
> +        }
> +        error_propagate(errp, err);
>          return;
>      }
>  
> 

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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to