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

[Peter Maydell]

On Tue, 31 Aug 2021 at 13:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Will look at it next week.

Ping?

thanks
-- PMM

> Paolo
>
> Il lun 30 ago 2021, 17:37 Peter Maydell <peter.maydell@linaro.org> ha scritto:
>>
>> 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