Re: [PULL 2/2] core/register: Specify instance_size in the TypeInfo

[Alistair Francis]

On Thu, Oct 1, 2020 at 9:05 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> On Thu, Oct 01, 2020 at 08:37:31AM -0700, Alistair Francis wrote:
> > On Tue, Sep 29, 2020 at 6:22 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > > On Tue, Sep 29, 2020 at 01:55:35PM +0100, Peter Maydell wrote:
> > > > On Sun, 27 Sep 2020 at 15:00, Alistair Francis 
> > > > <alistair.francis@wdc.com> wrote:
> > > > >
> > > > > Reported-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > > > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > > > Message-Id: 
> > > > > <4cf1beb7dafb9143c261d266557d3173bf160524.1598376594.git.alistair.francis@wdc.com>
> > > > > ---
> > > > > @@ -269,13 +258,18 @@ static RegisterInfoArray 
> > > > > *register_init_block(DeviceState *owner,
> > > > >          int index = rae[i].addr / data_size;
> > > > >          RegisterInfo *r = &ri[index];
> > > > >
> > > > > -        *r = (RegisterInfo) {
> > > > > -            .data = data + data_size * index,
> > > > > -            .data_size = data_size,
> > > > > -            .access = &rae[i],
> > > > > -            .opaque = owner,
> > > > > -        };
> > > > > -        register_init(r);
> > > > > +        if (data + data_size * index == 0 || !&rae[i]) {
> > > > > +            continue;
> > > >
> > > > Coverity thinks (CID 1432800) that this is dead code, because
> > > > "data + data_size * index" can never be NULL[*]. What was this
> > > > intending to test for ? (maybe data == NULL? Missing dereference
> > > > operator ?)
> > >
> > > I believe the original check in the old register_init() function
> > > were just to make the function more flexible by allowing NULL
> > > arguments, but it was always unnecessary.  We have 4 callers of
> > > register_init_block*() and neither rae or data are NULL on those
> > > calls.
> >
> > In this case *data is an array, I guess the idea was to try and catch
> > if somehow a point in the array was NULL?
>
> I don't understand what you mean.  The area pointed by data
> doesn't contain any pointers, just the register values.

Yeah, I don't think this was ever right.

The idea I guess was to make sure that r.data was not NULL, but unless
data was NULL it couldn't be.

Alistair

>
> >
> > I'll send a patch to remove the check.
>
> Thanks!
>
> --
> Eduardo
>