Re: [PATCH v2 1/2] ASoC: cs35l41: CS35L41 Boosted Smart Amplifier

[Charles Keepax <ckeepax@opensource.cirrus.com>]

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