[PATCH v4 0/2] ASoC: samsung: remove cppcheck warnings
v4: corrected Fixes tag Added added Krzysztof Kozlowski's r-v-b tag reverted patch2 to v2 since this is the agreement.
v3: Added Sylwester tag Rebased and squashed fix with initial patch which was merged at some point but can't be found in broonie/for-next (not sure what happened?) Corrected patch subjects to tm2_wm5110 Reverted second patch to initial v1, after agreement between Krzysztof and Sylwester
v2: added Krzysztof Kozlowski's tags added fix for first patch already merged as suggested by Krzysztof Kozlowski moved variable to lower scope in patch6
Pierre-Louis Bossart (2): ASoC: samsung: tm2_wm5110: check of of_parse return value ASoC: samsung: tm2_wm5110: remove shadowed variable
sound/soc/samsung/tm2_wm5110.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
cppcheck warning:
sound/soc/samsung/tm2_wm5110.c:605:6: style: Variable 'ret' is reassigned a value before the old one has been used. [redundantAssignment] ret = devm_snd_soc_register_component(dev, &tm2_component, ^ sound/soc/samsung/tm2_wm5110.c:554:7: note: ret is assigned ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller", ^ sound/soc/samsung/tm2_wm5110.c:605:6: note: ret is overwritten ret = devm_snd_soc_register_component(dev, &tm2_component, ^
The args is a stack variable, so it could have junk (uninitialized) therefore args.np could have a non-NULL and random value even though property was missing. Later could trigger invalid pointer dereference.
There's no need to check for args.np because args.np won't be initialized on errors.
Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2 board") Cc: stable@vger.kernel.org Suggested-by: Krzysztof Kozlowski krzk@kernel.org Reviewed-by: Krzysztof Kozlowski krzysztof.kozlowski@canonical.com Reviewed-by: Sylwester Nawrocki s.nawrocki@samsung.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/samsung/tm2_wm5110.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c index 9300fef9bf26..125e07f65d2b 100644 --- a/sound/soc/samsung/tm2_wm5110.c +++ b/sound/soc/samsung/tm2_wm5110.c @@ -553,7 +553,7 @@ static int tm2_probe(struct platform_device *pdev)
ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller", cells_name, i, &args); - if (!args.np) { + if (ret) { dev_err(dev, "i2s-controller property parse error: %d\n", i); ret = -EINVAL; goto dai_node_put;
cppcheck warning:
sound/soc/samsung/tm2_wm5110.c:552:26: style: Local variable 'args' shadows outer variable [shadowVariable] struct of_phandle_args args; ^ sound/soc/samsung/tm2_wm5110.c:504:25: note: Shadowed declaration struct of_phandle_args args; ^ sound/soc/samsung/tm2_wm5110.c:552:26: note: Shadow variable struct of_phandle_args args; ^ Move the top-level variable to the lower scope where it's needed.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/samsung/tm2_wm5110.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c index 125e07f65d2b..84c2c63d5a87 100644 --- a/sound/soc/samsung/tm2_wm5110.c +++ b/sound/soc/samsung/tm2_wm5110.c @@ -501,7 +501,6 @@ static int tm2_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct snd_soc_card *card = &tm2_card; struct tm2_machine_priv *priv; - struct of_phandle_args args; struct snd_soc_dai_link *dai_link; int num_codecs, ret, i;
@@ -585,6 +584,8 @@ static int tm2_probe(struct platform_device *pdev) }
if (num_codecs > 1) { + struct of_phandle_args args; + /* HDMI DAI link (I2S1) */ i = card->num_links - 1;
On 12/03/2021 19:02, Pierre-Louis Bossart wrote:
cppcheck warning:
sound/soc/samsung/tm2_wm5110.c:552:26: style: Local variable 'args' shadows outer variable [shadowVariable] struct of_phandle_args args; ^ sound/soc/samsung/tm2_wm5110.c:504:25: note: Shadowed declaration struct of_phandle_args args; ^ sound/soc/samsung/tm2_wm5110.c:552:26: note: Shadow variable struct of_phandle_args args; ^ Move the top-level variable to the lower scope where it's needed.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/samsung/tm2_wm5110.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Thanks! Reviewed-by: Krzysztof Kozlowski krzysztof.kozlowski@canonical.com
Best regards, Krzysztof
On Fri, 12 Mar 2021 12:02:29 -0600, Pierre-Louis Bossart wrote:
v4: corrected Fixes tag Added added Krzysztof Kozlowski's r-v-b tag reverted patch2 to v2 since this is the agreement.
v3: Added Sylwester tag Rebased and squashed fix with initial patch which was merged at some point but can't be found in broonie/for-next (not sure what happened?) Corrected patch subjects to tm2_wm5110 Reverted second patch to initial v1, after agreement between Krzysztof and Sylwester
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: samsung: tm2_wm5110: check of of_parse return value commit: d58970da324732686529655c21791cef0ee547c4 [2/2] ASoC: samsung: tm2_wm5110: remove shadowed variable commit: f7b61287cf17486dd09438115a993d699db2ab3b
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
Krzysztof Kozlowski -
Mark Brown -
Pierre-Louis Bossart