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: Wed, 5 Feb 2020 08:50:06 -0700 From: Nathan Chancellor <natechancellor@...il.com> To: Mark Brown <broonie@...nel.org> Cc: Liam Girdwood <lgirdwood@...il.com>, Srinivas Kandagatla <srinivas.kandagatla@...aro.org>, alsa-devel@...a-project.org, linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com Subject: Re: [PATCH] ASoC: wcd934x: Remove some unnecessary NULL checks On Wed, Feb 05, 2020 at 10:22:38AM +0000, Mark Brown wrote: > On Tue, Feb 04, 2020 at 12:32:15PM -0700, Nathan Chancellor wrote: > > On Tue, Feb 04, 2020 at 10:00:39AM +0000, Mark Brown wrote: > > > > I'm not convincd this is a sensible warning, at the use site a > > > pointer to an array in a struct looks identical to an array > > > embedded in the struct so it's not such a bad idea to check and > > > refactoring of the struct could easily introduce problems. > > > Other static checkers like smatch will warn about this as well (since I > > am sure that is how Dan Carpenter found the same issue in the wcd9335 > > driver). Isn't an antipattern in the kernel to do things "just in > > case we do something later"? There are plenty of NULL checks removed > > from the kernel because they do not do anything now. > > I'm not convinced it is an antipattern - adding the checks would > be a bit silly but with the way C works the warnings feel like > false positives. If the compiler were able to warn about missing > NULL checks in the case where the thing in the struct is a > pointer I'd be a lot happier with this. Yes, that would definitely be nice. I am not entirely sure that this is possible with clang due to its architecture but I am far from a clang internal expert. > > I'd be fine with changing the check to something else that keeps the > > same logic but doesn't create a warning; I am not exactly sure what that > > would be because that is more of a specific driver logic thing, which I > > am not familiar with. > > I've queued the change to be applied since it's shuts the > compiler up but I'm really not convinced the compiler is helping > here. Thank you :) Cheers, Nathan
Powered by blists - more mailing lists