[PATCH 0/6] ASoC: core: remove cppcheck warnings

[PATCH 0/6] ASoC: core: remove cppcheck warnings

[Pierre-Louis Bossart]

This is the first batch of cleanups to make cppcheck more usable,
currently we have way too many warnings that drown real issues.

Pierre-Louis Bossart (6):
  ASoC: soc-ops: remove useless assignment
  ASoC: soc-pcm: remove redundant assignment
  ASoC: soc-pcm: remove shadowing variable
  ASoC: soc-pcm: add error log
  ASoC: soc-topology: clarify expression
  ASoC: generic: simple-card-utils: remove useless assignment

 sound/soc/generic/simple-card-utils.c |  2 +-
 sound/soc/soc-ops.c                   |  2 +-
 sound/soc/soc-pcm.c                   |  4 ++--
 sound/soc/soc-topology.c              | 16 ++++++++--------
 4 files changed, 12 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH 1/6] ASoC: soc-ops: remove useless assignment

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/soc-ops.c:410:35: style: Variable 'val2' is assigned a value
that is never used. [unreadVariable]
 unsigned int val, val_mask, val2 = 0;
                                  ^

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index 10f48827bb0e..58527247df83 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -407,7 +407,7 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
 	int min = mc->min;
 	unsigned int mask = (1U << (fls(min + max) - 1)) - 1;
 	int err = 0;
-	unsigned int val, val_mask, val2 = 0;
+	unsigned int val, val_mask, val2;
 
 	val_mask = mask << shift;
 	val = (ucontrol->value.integer.value[0] + min) & mask;
-- 
2.25.1

[PATCH 2/6] ASoC: soc-pcm: remove redundant assignment

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/soc-pcm.c:2398:7: style: Variable 'ret' is reassigned a
value before the old one has been used. [redundantAssignment]
  ret = -EINVAL;
      ^
sound/soc/soc-pcm.c:2395:7: note: ret is assigned
  ret = -EINVAL;
      ^
sound/soc/soc-pcm.c:2398:7: note: ret is overwritten
  ret = -EINVAL;
      ^

This looks like a copy/paste or git merge issue.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-pcm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 14d85ca1e435..12fd10a6c190 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2395,7 +2395,6 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
 	/* Only start the BE if the FE is ready */
 	if (fe->dpcm[stream].state == SND_SOC_DPCM_STATE_HW_FREE ||
 		fe->dpcm[stream].state == SND_SOC_DPCM_STATE_CLOSE) {
-		ret = -EINVAL;
 		dev_err(fe->dev, "ASoC: FE %s is not ready %d\n",
 			fe->dai_link->name, fe->dpcm[stream].state);
 		ret = -EINVAL;
-- 
2.25.1

[PATCH 3/6] ASoC: soc-pcm: remove shadowing variable

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/soc-pcm.c:1718:7: style: Local variable 'i' shadows outer
variable [shadowVariable]
  int i;
      ^
sound/soc/soc-pcm.c:1696:6: note: Shadowed declaration
 int i;
     ^
sound/soc/soc-pcm.c:1718:7: note: Shadow variable
  int i;
      ^

the second variable seems totally unnecessary.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-pcm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 12fd10a6c190..705fb2d548a9 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1718,7 +1718,6 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
 			snd_soc_dpcm_get_substream(be, stream);
 		struct snd_soc_pcm_runtime *rtd;
 		struct snd_soc_dai *dai;
-		int i;
 
 		/* A backend may not have the requested substream */
 		if (!be_substream)
-- 
2.25.1

[PATCH 4/6] ASoC: soc-pcm: add error log

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/soc-pcm.c:1907:6: style: Variable 'err' is assigned a value
that is never used. [unreadVariable]
 err = dpcm_be_dai_hw_free(fe, stream);
     ^

it's not clear why dpcm_be_dai_hw_free() is sometimes called without
testing the error status, and sometimes an error message is provided.

When in doubt, add an error message for consistency. This may have to
be revisited.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-pcm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 705fb2d548a9..2de69dc240bd 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1907,6 +1907,8 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream)
 	/* only hw_params backends that are either sinks or sources
 	 * to this frontend DAI */
 	err = dpcm_be_dai_hw_free(fe, stream);
+	if (err < 0)
+		dev_err(fe->dev, "ASoC: hw_free BE failed %d\n", err);
 
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE;
 	dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
-- 
2.25.1

[PATCH 5/6] ASoC: soc-topology: clarify expression

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/soc-topology.c:1676:52: style: Clarify calculation
precedence for '&' and '?'. [clarifyCalculation]
   flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_RATES ? 1 : 0;
                                                   ^
sound/soc/soc-topology.c:1680:55: style: Clarify calculation
precedence for '&' and '?'. [clarifyCalculation]
   flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_CHANNELS ?
                                                      ^
sound/soc/soc-topology.c:1685:57: style: Clarify calculation
precedence for '&' and '?'. [clarifyCalculation]
   flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_SAMPLEBITS ?
                                                        ^
sound/soc/soc-topology.c:1768:52: style: Clarify calculation
precedence for '&' and '?'. [clarifyCalculation]
   flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_RATES ? 1 : 0;
                                                   ^
sound/soc/soc-topology.c:1772:55: style: Clarify calculation
precedence for '&' and '?'. [clarifyCalculation]
   flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_CHANNELS ?
                                                      ^
sound/soc/soc-topology.c:1777:57: style: Clarify calculation
precedence for '&' and '?'. [clarifyCalculation]
   flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_SAMPLEBITS ?
                                                        ^
sound/soc/soc-topology.c:1782:48: style: Clarify calculation
precedence for '&' and '?'. [clarifyCalculation]
  flags & SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP ?
                                               ^

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 1b0cd33a1348..73076d425efb 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1673,16 +1673,16 @@ static void set_dai_flags(struct snd_soc_dai_driver *dai_drv,
 {
 	if (flag_mask & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_RATES)
 		dai_drv->symmetric_rate =
-			flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_RATES ? 1 : 0;
+			(flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_RATES) ? 1 : 0;
 
 	if (flag_mask & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_CHANNELS)
 		dai_drv->symmetric_channels =
-			flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_CHANNELS ?
+			(flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_CHANNELS) ?
 			1 : 0;
 
 	if (flag_mask & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_SAMPLEBITS)
 		dai_drv->symmetric_sample_bits =
-			flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_SAMPLEBITS ?
+			(flags & SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_SAMPLEBITS) ?
 			1 : 0;
 }
 
@@ -1765,22 +1765,22 @@ static void set_link_flags(struct snd_soc_dai_link *link,
 {
 	if (flag_mask & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_RATES)
 		link->symmetric_rate =
-			flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_RATES ? 1 : 0;
+			(flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_RATES) ? 1 : 0;
 
 	if (flag_mask & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_CHANNELS)
 		link->symmetric_channels =
-			flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_CHANNELS ?
+			(flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_CHANNELS) ?
 			1 : 0;
 
 	if (flag_mask & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_SAMPLEBITS)
 		link->symmetric_sample_bits =
-			flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_SAMPLEBITS ?
+			(flags & SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_SAMPLEBITS) ?
 			1 : 0;
 
 	if (flag_mask & SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP)
 		link->ignore_suspend =
-		flags & SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP ?
-		1 : 0;
+			(flags & SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP) ?
+			1 : 0;
 }
 
 /* create the FE DAI link */
-- 
2.25.1

[PATCH 6/6] ASoC: generic: simple-card-utils: remove useless assignment

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/generic/simple-card-utils.c:258:10: style: Variable 'ret' is
assigned a value that is never used. [unreadVariable]
 int ret = 0;
         ^

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/generic/simple-card-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index ab31045cfc95..06c2512b6f2d 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -254,7 +254,7 @@ int asoc_simple_hw_params(struct snd_pcm_substream *substream,
 	struct simple_dai_props *dai_props =
 		simple_priv_to_props(priv, rtd->num);
 	unsigned int mclk, mclk_fs = 0;
-	int ret = 0;
+	int ret;
 
 	if (dai_props->mclk_fs)
 		mclk_fs = dai_props->mclk_fs;
-- 
2.25.1

Re: [PATCH 0/6] ASoC: core: remove cppcheck warnings

[Mark Brown]

On Thu, 18 Feb 2021 16:19:15 -0600, Pierre-Louis Bossart wrote:
> This is the first batch of cleanups to make cppcheck more usable,
> currently we have way too many warnings that drown real issues.
> 
> Pierre-Louis Bossart (6):
>   ASoC: soc-ops: remove useless assignment
>   ASoC: soc-pcm: remove redundant assignment
>   ASoC: soc-pcm: remove shadowing variable
>   ASoC: soc-pcm: add error log
>   ASoC: soc-topology: clarify expression
>   ASoC: generic: simple-card-utils: remove useless assignment
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/6] ASoC: soc-ops: remove useless assignment
      commit: 56dc057925b112353a4d920380c537d1f96699a0
[2/6] ASoC: soc-pcm: remove redundant assignment
      commit: 8f7351ec37b52d22e77d2cab38ddd4aa920af0b4
[3/6] ASoC: soc-pcm: remove shadowing variable
      commit: 52fcd9638da0803c6fe0cfadab7af978c961be37
[4/6] ASoC: soc-pcm: add error log
      commit: 56fc1a7fd01ef0984d0272e52a9823ca11eff890
[5/6] ASoC: soc-topology: clarify expression
      commit: 761fa730a2e1e9197d89f3e9d1a13a9be165b109
[6/6] ASoC: generic: simple-card-utils: remove useless assignment
      commit: 8754b443fa7df24e357b7e707c901eefe373a05c

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