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

[Peter Maydell]

On Mon, 9 Aug 2021 at 11:40, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> 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).

Ping? It would be nice to either confirm this is a false
positive or else fix it.

> > +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