| Message ID | 20210222213306.22654-7-pierre-louis.bossart@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | f7b61287cf17486dd09438115a993d699db2ab3b |
| Headers | show |
| Series | ASoC: samsung: remove cppcheck warnings | expand |
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
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.
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 --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;
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(-)