diff mbox series

[v2,6/6] ASoC: samsung: tm2_wm5510: remove shadowed variable

Message ID 20210222213306.22654-7-pierre-louis.bossart@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series ASoC: samsung: remove cppcheck warnings | expand

Commit Message

Pierre-Louis Bossart Feb. 22, 2021, 9:33 p.m. UTC
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(-)

Comments

Sylwester Nawrocki Feb. 23, 2021, 11:25 a.m. UTC | #1
On 22.02.2021 22:33, Pierre-Louis Bossart wrote:
> Move the top-level variable to the lower scope where it's needed.

Actually I like your original patch better as there is really no need
for multiple lower scope declarations in that fairly small function.
Pierre-Louis Bossart Feb. 23, 2021, 3:13 p.m. UTC | #2
On 2/23/21 5:25 AM, Sylwester Nawrocki wrote:
> On 22.02.2021 22:33, Pierre-Louis Bossart wrote:
>> Move the top-level variable to the lower scope where it's needed.
> 
> Actually I like your original patch better as there is really no need
> for multiple lower scope declarations in that fairly small function.

I have no opinion, just let me know what the consensus is.
Krzysztof Kozlowski Feb. 23, 2021, 6:29 p.m. UTC | #3
On Tue, 23 Feb 2021 at 19:20, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 2/23/21 5:25 AM, Sylwester Nawrocki wrote:
> > On 22.02.2021 22:33, Pierre-Louis Bossart wrote:
> >> Move the top-level variable to the lower scope where it's needed.
> >
> > Actually I like your original patch better as there is really no need
> > for multiple lower scope declarations in that fairly small function.
>
> I have no opinion, just let me know what the consensus is.

I proposed to have both variables local scope, to reduce the size of
function-scope variables. Their number tends to grow in probe() a lot,
so when a variable can be localized more, it makes the code easier to
understand. No need to figure out who/where/when uses the variable.
Local scope makes it much easier.

Best regards,
Krzysztof
Sylwester Nawrocki Feb. 23, 2021, 7:24 p.m. UTC | #4
On 23.02.2021 19:29, Krzysztof Kozlowski wrote:
> On Tue, 23 Feb 2021 at 19:20, Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>> On 2/23/21 5:25 AM, Sylwester Nawrocki wrote:
>>> On 22.02.2021 22:33, Pierre-Louis Bossart wrote:
>>>> Move the top-level variable to the lower scope where it's needed.
>>>
>>> Actually I like your original patch better as there is really no need
>>> for multiple lower scope declarations in that fairly small function.
>>
>> I have no opinion, just let me know what the consensus is.
> 
> I proposed to have both variables local scope, to reduce the size of
> function-scope variables. Their number tends to grow in probe() a lot,
> so when a variable can be localized more, it makes the code easier to
> understand. No need to figure out who/where/when uses the variable.
> Local scope makes it much easier.

I don't have strong opinion, let's keep it local then as in current patch.
Krzysztof Kozlowski Feb. 23, 2021, 7:41 p.m. UTC | #5
On Mon, Feb 22, 2021 at 03:33:06PM -0600, 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(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof
diff mbox series

Patch

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;