On Fri, Jul 02, 2021 at 10:23:52AM -0500, David Rhodes wrote:
> On 7/2/21 7:08 AM, Mark Brown wrote:
> >On Thu, Jul 01, 2021 at 03:22:53PM -0500, David Rhodes wrote:
> >>On 6/29/21 6:51 PM, Pierre-Louis Bossart wrote:
> On 6/29/21 5:27 PM, David Rhodes wrote:
> > + unsigned int status[4] = {0, 0, 0, 0};
> > + unsigned int masks[4] = {0, 0, 0, 0};
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(status); i++) {
> > + regmap_read(cs35l41->regmap,
> > + CS35L41_IRQ1_STATUS1 + (i * CS35L41_REGSTRIDE),
> > + &status[i]);
> > + regmap_read(cs35l41->regmap,
> > + CS35L41_IRQ1_MASK1 + (i * CS35L41_REGSTRIDE),
> > + &masks[i]);
> > + }
>
> Pierre is correct that both arrays will always be fully initialized
> with regmap reads in the loop before any other access.
> Without the explicit initialization, an analyzer that is not as
> smart as Pierre will tell me that the array values can be used
> uninitialized later on.
> Presumably the tool does not unroll the loop to see that every array
> value is assigned.
>
> So I don't think I have an actual issue, but I am just shutting up
> the static analyzers, although I do think I fixed what they were
> telling me.
This isn't quite correct the static analyser is flagging a real
issue here. The issue is because the return values from the
regmap_reads are not checked, if regmap_read fails then that
index in the array would not be initialised and then gets used
later. Now if the read failing isn't a situation the drive is
bothered about initalising to zero is probably fine. So I guess it
doesn't feel to me that the code particularly needs changed here,
but certainly the warning if you remove the initialisations is real.
Thanks,
Charles
