Re: [PULL 8/9] qapi: introduce forwarding visitor

[Peter Maydell]

On Sat, 24 Jul 2021 at 10:00, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> This new adaptor visitor takes a single field of the adaptee, and exposes it
> with a different name.
>
> This will be used for QOM alias properties.  Alias targets can of course
> have a different name than the alias property itself (e.g. a machine's
> pflash0 might be an alias of a property named 'drive').  When the target's
> getter or setter invokes the visitor, it will use a different name than
> what the caller expects, and the visitor will not be able to find it
> (or will consume erroneously).
>
> The solution is for alias getters and setters to wrap the incoming
> visitor, and forward the sole field that the target is expecting while
> renaming it appropriately.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Hi; Coverity complains here (CID 1459068) that the call to
visit_optional() is ignoring its return value (which we check
in 983 out of the other 989 callsites).

> +static void forward_field_optional(Visitor *v, const char *name, bool 
> *present)
> +{
> +    ForwardFieldVisitor *ffv = to_ffv(v);
> +
> +    if (!forward_field_translate_name(ffv, &name, NULL)) {
> +        *present = false;
> +        return;
> +    }
> +    visit_optional(ffv->target, name, present);
> +}

Is it right, or is this its "looks like this is returning an error
indication" heuristic misfiring again ?

My guess is the latter and it's caused by a mismatch
between the prototype of visit_optional() (returns a
status both by setting *present and in its return value)
and the Visitor::optional method (returns a status only
by setting *present, return void). I guess ideally we'd
standardize on whether these things were intended to return
a value or not.

thanks
-- PMM