[PATCH 0/6] ASoC: core: remove cppcheck warnings
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(-)
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;
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;
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)
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);
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 */
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;
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
participants (2)
-
Mark Brown -
Pierre-Louis Bossart