lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 23 Aug 2022 08:35:26 +1000 From: Matt Flax <flatmax@...tmax.com> To: Nathan Chancellor <nathan@...nel.org> Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>, Tom Rix <trix@...hat.com>, alsa-devel@...a-project.org, linux-kernel@...r.kernel.org, llvm@...ts.linux.dev, patches@...ts.linux.dev, kernel test robot <lkp@...el.com>, "Sudip Mukherjee (Codethink)" <sudipm.mukherjee@...il.com> Subject: Re: [PATCH] ASoC: codes: src4xxx: Avoid clang -Wsometimes-uninitialized in src4xxx_hw_params() That solution looks good. Matt On 23/8/22 08:32, Nathan Chancellor wrote: > Hi Matt, > > On Tue, Aug 23, 2022 at 08:00:07AM +1000, Matt Flax wrote: >> Hi Nathan, >> >> On 23/8/22 04:31, Nathan Chancellor wrote: >>> Clang warns: >>> >>> sound/soc/codecs/src4xxx.c:280:3: error: variable 'd' is used uninitialized whenever switch default is taken [-Werror,-Wsometimes-uninitialized] >>> default: >>> ^~~~~~~ >>> sound/soc/codecs/src4xxx.c:298:59: note: uninitialized use occurs here >>> ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_11, d); >>> ^ >>> sound/soc/codecs/src4xxx.c:223:20: note: initialize the variable 'd' to silence this warning >>> int val, pj, jd, d; >>> ^ >>> = 0 >> >> If you really want to get rid of these warnings, you can use this default >> for the variables : >> >> pj = 0x0; >> jd=0xff; >> >> d = 0xff; >> >> It doesn't really make sense why we would choose to initialise these >> variables, but if you want to silence the preprocessor, then perhaps those >> values. Put a message that defaults are not known nor specified in the data >> sheet and these values are chosen to be unlikely matches of real world >> values - which ensures regmap updates later from an initial unknown chip >> state after refresh. > Okay, I could do something like this? > > diff --git a/sound/soc/codecs/src4xxx.c b/sound/soc/codecs/src4xxx.c > index a8f143057b41..db4e280dd055 100644 > --- a/sound/soc/codecs/src4xxx.c > +++ b/sound/soc/codecs/src4xxx.c > @@ -280,9 +280,14 @@ static int src4xxx_hw_params(struct snd_pcm_substream *substream, > default: > /* don't error out here, > * other parts of the chip are still functional > + * Dummy initialize variables to avoid > + * -Wsometimes-uninitialized from clang. > */ > dev_info(component->dev, > - "Couldn't set the RCV PLL as this master clock rate is unknown\n"); > + "Couldn't set the RCV PLL as this master clock rate is unknown. Chosen regmap values may not match real world values.\n"); > + pj = 0x0; > + jd = 0xff; > + d = 0xff; > break; > } > ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_0F, pj); > > These warnings need to be eliminated because Linus requires a clean > build as much as possible [1] [2], which has been codified by > CONFIG_WERROR, which is enabled by default with allmodconfig. As a > result, clang allmodconfig builds are broken [3], which you might have > already seen. If there is another way this could be written without > potentially uninitialized variables, I am open to it. > > [1]: https://lore.kernel.org/CAHk-=wg-mH-_GYpkhz_psjBWG6ZcjKnPo83fg7YMj_by+-LRTQ@mail.gmail.com/ > [2]: https://lore.kernel.org/CAHk-=wi4wyvuXs0SUq-2x=XHxWmJ6jVKRD-WpE0kWgWiqVJNbg@mail.gmail.com/ > [3]: https://lore.kernel.org/YwNMUlAmu%2FqCjuva@debian/ > >> All a bit messy really. > Indeed :/ > >>> According to the comment in the default case, other parts of the chip >>> are still functional without these values so just return 0 in the >>> default case to avoid using these variables uninitialized. >>> >>> Link: https://github.com/ClangBuiltLinux/linux/issues/1691 >>> Reported-by: kernel test robot <lkp@...el.com> >>> Reported-by: "Sudip Mukherjee (Codethink)" <sudipm.mukherjee@...il.com> >>> Signed-off-by: Nathan Chancellor <nathan@...nel.org> >>> --- >>> sound/soc/codecs/src4xxx.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/sound/soc/codecs/src4xxx.c b/sound/soc/codecs/src4xxx.c >>> index a8f143057b41..cf45caa4bf7f 100644 >>> --- a/sound/soc/codecs/src4xxx.c >>> +++ b/sound/soc/codecs/src4xxx.c >>> @@ -283,7 +283,7 @@ static int src4xxx_hw_params(struct snd_pcm_substream *substream, >>> */ >>> dev_info(component->dev, >>> "Couldn't set the RCV PLL as this master clock rate is unknown\n"); >>> - break; >>> + return 0; >> >> Don't return here, the rest of the chip is still functional and probably in >> use. Print the dev_info and continue. > I'll send a v2 tomorrow morning (for me at least) if there are no other > comments. > > Cheers, > Nathan
Powered by blists - more mailing lists