[PATCH v3 0/2] ASoC: samsung: remove cppcheck warnings
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 shadowing variable
sound/soc/samsung/tm2_wm5110.c | 3 +-- 1 file changed, 1 insertion(+), 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 TM2board") Cc: stable@vger.kernel.org Suggested-by: Krzysztof Kozlowski krzk@kernel.org 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;
On 11/03/2021 01:35, Pierre-Louis Bossart wrote:
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 TM2board") Cc: stable@vger.kernel.org Suggested-by: Krzysztof Kozlowski krzk@kernel.org 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(-)
Reviewed-by: Krzysztof Kozlowski krzysztof.kozlowski@canonical.com
Best regards, Krzysztof
On Wed, Mar 10, 2021 at 06:35:15PM -0600, Pierre-Louis Bossart wrote:
Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board") Cc: stable@vger.kernel.org
Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value") Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board") Has these problem(s): - Subject does not match target commit subject Just use git log -1 --format='Fixes: %h ("%s")'
On 3/12/21 8:28 AM, Mark Brown wrote:
On Wed, Mar 10, 2021 at 06:35:15PM -0600, Pierre-Louis Bossart wrote:
Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board") Cc: stable@vger.kernel.org
Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value") Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board") Has these problem(s): - Subject does not match target commit subject Just use git log -1 --format='Fixes: %h ("%s")'
Sorry, I don't know what to make of this. I don't see this commit 11bc3bb24003
Something odd happened, there was an initial merge and it seems to have disappeared, it's no longer in the for-next branch?
On Fri, Mar 12, 2021 at 10:30:32AM -0600, Pierre-Louis Bossart wrote:
On 3/12/21 8:28 AM, Mark Brown wrote:
Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value") Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board") Has these problem(s): - Subject does not match target commit subject Just use git log -1 --format='Fixes: %h ("%s")'
Sorry, I don't know what to make of this. I don't see this commit 11bc3bb24003
Something odd happened, there was an initial merge and it seems to have disappeared, it's no longer in the for-next branch?
That commit is your patch being applied, which I've dropped because of the error reported.
On 3/12/21 10:31 AM, Mark Brown wrote:
On Fri, Mar 12, 2021 at 10:30:32AM -0600, Pierre-Louis Bossart wrote:
On 3/12/21 8:28 AM, Mark Brown wrote:
Commit: 11bc3bb24003 ("ASoC: samsung: tm2_wm5110: check of of_parse return value") Fixes tag: Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2board") Has these problem(s): - Subject does not match target commit subject Just use git log -1 --format='Fixes: %h ("%s")'
Sorry, I don't know what to make of this. I don't see this commit 11bc3bb24003
Something odd happened, there was an initial merge and it seems to have disappeared, it's no longer in the for-next branch?
That commit is your patch being applied, which I've dropped because of the error reported.
ack, not sure why there's a missing space.
git log -1 --format='Fixes: %h ("%s")' 8d1513cef51a Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2 board")
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; ^
it's not clear why there was a need for a local variable at a lower scope, remove it and share the same variable between scopes.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/samsung/tm2_wm5110.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c index 125e07f65d2b..c1ab6c3afd0e 100644 --- a/sound/soc/samsung/tm2_wm5110.c +++ b/sound/soc/samsung/tm2_wm5110.c @@ -549,7 +549,6 @@ static int tm2_probe(struct platform_device *pdev) }
for (i = 0; i < num_codecs; i++) { - struct of_phandle_args args;
ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller", cells_name, i, &args);
On 11/03/2021 01:35, 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; ^
it's not clear why there was a need for a local variable at a lower scope, remove it and share the same variable between scopes.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/samsung/tm2_wm5110.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c index 125e07f65d2b..c1ab6c3afd0e 100644 --- a/sound/soc/samsung/tm2_wm5110.c +++ b/sound/soc/samsung/tm2_wm5110.c @@ -549,7 +549,6 @@ static int tm2_probe(struct platform_device *pdev) }
for (i = 0; i < num_codecs; i++) {
struct of_phandle_args args;ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller", cells_name, i, &args);
The agreement was to keep it as in v2, so to have two local variables instead of bigger scope.
Best regards, Krzysztof
On 3/12/21 4:45 AM, Krzysztof Kozlowski wrote:
On 11/03/2021 01:35, 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; ^
it's not clear why there was a need for a local variable at a lower scope, remove it and share the same variable between scopes.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/samsung/tm2_wm5110.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c index 125e07f65d2b..c1ab6c3afd0e 100644 --- a/sound/soc/samsung/tm2_wm5110.c +++ b/sound/soc/samsung/tm2_wm5110.c @@ -549,7 +549,6 @@ static int tm2_probe(struct platform_device *pdev) }
for (i = 0; i < num_codecs; i++) {
struct of_phandle_args args;ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller", cells_name, i, &args);
The agreement was to keep it as in v2, so to have two local variables instead of bigger scope.
Ack, I misunderstood the feedback, will resend.
On Wed, 10 Mar 2021 18:35:14 -0600, Pierre-Louis Bossart wrote:
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
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