From e185dffda8da9d2de2df29cf1905264fc1d61072 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Thu, 26 Aug 2021 16:22:38 -0700 Subject: [PATCH 01/10] init: fix uninitiallized variable we should never need it, but this is just to silence cppcheck Signed-off-by: Curtis Malainey --- src/init/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/init/init.c b/src/init/init.c index eb379746129f..0b3f08e0f490 100644 --- a/src/init/init.c +++ b/src/init/init.c @@ -162,7 +162,7 @@ static int primary_core_init(int argc, char *argv[], struct sof *sof) #ifndef __ZEPHYR__ int main(int argc, char *argv[]) { - int err; + int err = 0; trace_point(TRACE_BOOT_START); From 0a906e2ae26b95978c9bbb486a595a26fe5aa5b5 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Thu, 26 Aug 2021 16:28:41 -0700 Subject: [PATCH 02/10] dmic_nhlt: fix uninitialized value cppcheck error Signed-off-by: Curtis Malainey --- src/drivers/intel/dmic/dmic_nhlt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/intel/dmic/dmic_nhlt.c b/src/drivers/intel/dmic/dmic_nhlt.c index ad39a32e6714..d8d18453e106 100644 --- a/src/drivers/intel/dmic/dmic_nhlt.c +++ b/src/drivers/intel/dmic/dmic_nhlt.c @@ -189,7 +189,7 @@ int dmic_set_config_nhlt(struct dai *dai, void *spec_config) uint32_t channel_ctrl_mask; uint32_t fir_control; uint32_t pdm_ctrl_mask; - uint32_t ref; + uint32_t ref = 0; uint32_t val; const uint8_t *p = spec_config; int num_fifos; From fb20fade6836e0646d139f3182b6e6261d8079a8 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Thu, 26 Aug 2021 16:39:06 -0700 Subject: [PATCH 03/10] multiband_drc: fix null pointer check cppcheck pointed out that either our nullpointer check was not needed or that we are failing to do it before using the pointer. The check looks valid so lets fix the lookup. Signed-off-by: Curtis Malainey --- src/audio/multiband_drc/multiband_drc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/audio/multiband_drc/multiband_drc.c b/src/audio/multiband_drc/multiband_drc.c index 6ce23f60b90c..2c021ea14985 100644 --- a/src/audio/multiband_drc/multiband_drc.c +++ b/src/audio/multiband_drc/multiband_drc.c @@ -108,14 +108,15 @@ static int multiband_drc_init_coef(struct multiband_drc_comp_data *cd, int16_t n struct sof_multiband_drc_config *config = cd->config; struct multiband_drc_state *state = &cd->state; uint32_t sample_bytes = get_sample_bytes(cd->source_format); - int num_bands = cd->config->num_bands; - int i, ch, ret; + int i, ch, ret, num_bands; if (!config) { comp_cl_err(&comp_multiband_drc, "multiband_drc_init_coef(), no config is set"); return -EINVAL; } + num_bands = config->num_bands; + /* Sanity checks */ if (nch > PLATFORM_MAX_CHANNELS) { comp_cl_err(&comp_multiband_drc, From 609b5c18004d3515de80713a30cc34a4244cd373 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Tue, 21 Sep 2021 15:57:25 -0700 Subject: [PATCH 04/10] numbers: fix signed int overflow if we shift 1 left 31 times we will overflow the signed bit. Assuming the intent is correct lets make this unsigned so cppcheck won't complain. Signed-off-by: Curtis Malainey --- src/include/sof/math/numbers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/sof/math/numbers.h b/src/include/sof/math/numbers.h index 5c527b0b8382..d4bfe82a55ac 100644 --- a/src/include/sof/math/numbers.h +++ b/src/include/sof/math/numbers.h @@ -48,7 +48,7 @@ static inline int ceil_divide(int a, int b) * If the signs are the same, we check if there was any remainder in * the division by multiplying the number back. */ - if (!((a ^ b) & (1 << ((sizeof(int) * 8) - 1))) && c * b != a) + if (!((a ^ b) & (1U << ((sizeof(int) * 8) - 1))) && c * b != a) c++; return c; From 28a590f4bb296976963fdf6d3e8c36ab88ea6700 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Tue, 21 Sep 2021 16:19:22 -0700 Subject: [PATCH 05/10] dmic: change condition to satisfy cppcheck cppcheck for some reason doesn't understand that j > 1 has nothing to do with the indexing itself and this the check is pointless, so switch to != so it doesn't flag the issue anymore. Signed-off-by: Curtis Malainey --- src/drivers/intel/dmic/dmic_computed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/intel/dmic/dmic_computed.c b/src/drivers/intel/dmic/dmic_computed.c index f6dd39bf8bbe..7d4ffef2a635 100644 --- a/src/drivers/intel/dmic/dmic_computed.c +++ b/src/drivers/intel/dmic/dmic_computed.c @@ -132,7 +132,7 @@ static void find_modes(struct dai *dai, mfir = fir_list[j]->decim_factor; /* Skip if previous decimation factor was the same */ - if (j > 1 && fir_list[j - 1]->decim_factor == mfir) + if (j != 0 && fir_list[j - 1]->decim_factor == mfir) continue; mcic = osr / mfir; From ad960cc464aa0106176934cc7b06923a97460cbf Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Tue, 21 Sep 2021 16:29:32 -0700 Subject: [PATCH 06/10] codec_adapter: remove pointless null pointer check We should always have a dev, and even if we didn't we would crash here because we logged above in debug using dev and we are logging the error itself using dev. Signed-off-by: Curtis Malainey --- src/audio/codec_adapter/codec/generic.c | 2 +- src/audio/codec_adapter/codec_adapter.c | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/audio/codec_adapter/codec/generic.c b/src/audio/codec_adapter/codec/generic.c index 179cf082088c..e7f97da9e287 100644 --- a/src/audio/codec_adapter/codec/generic.c +++ b/src/audio/codec_adapter/codec/generic.c @@ -29,7 +29,7 @@ codec_load_config(struct comp_dev *dev, void *cfg, size_t size, comp_dbg(dev, "codec_load_config() start"); - if (!dev || !cfg || !size) { + if (!cfg || !size) { comp_err(dev, "codec_load_config(): wrong input params! dev %x, cfg %x size %d", (uint32_t)dev, (uint32_t)cfg, size); return -EINVAL; diff --git a/src/audio/codec_adapter/codec_adapter.c b/src/audio/codec_adapter/codec_adapter.c index 9891eb9e24d9..1b8a9aebb293 100644 --- a/src/audio/codec_adapter/codec_adapter.c +++ b/src/audio/codec_adapter/codec_adapter.c @@ -45,7 +45,7 @@ struct comp_dev *codec_adapter_new(const struct comp_driver *drv, comp_cl_dbg(drv, "codec_adapter_new() start"); - if (!drv || !config) { + if (!config) { comp_cl_err(drv, "codec_adapter_new(), wrong input params! drv = %x config = %x", (uint32_t)drv, (uint32_t)config); return NULL; @@ -123,11 +123,7 @@ int load_setup_config(struct comp_dev *dev, void *cfg, uint32_t size) comp_dbg(dev, "load_setup_config() start."); - if (!dev) { - comp_err(dev, "load_setup_config(): no component device."); - ret = -EINVAL; - goto end; - } else if (!cfg || !size) { + if (!cfg || !size) { comp_err(dev, "load_setup_config(): no config available cfg: %x, size: %d", (uintptr_t)cfg, size); ret = -EINVAL; From 9b04a7305118feeece9776dc213016e57138eeac Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Tue, 21 Sep 2021 16:31:46 -0700 Subject: [PATCH 07/10] crossover: fix return type Can't return a negative error if we are returning an unsigned type. C is great right? No reason to ever use something like rust... Signed-off-by: Curtis Malainey --- src/audio/crossover/crossover.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/audio/crossover/crossover.c b/src/audio/crossover/crossover.c index 633a9eba4261..f2a9966107b8 100644 --- a/src/audio/crossover/crossover.c +++ b/src/audio/crossover/crossover.c @@ -94,7 +94,7 @@ static inline void crossover_reset_state(struct comp_data *cd) * \return the position at which pipe_id is found in config->assign_sink. * -EINVAL if not found. */ -static uint8_t crossover_get_stream_index(struct sof_crossover_config *config, +static int crossover_get_stream_index(struct sof_crossover_config *config, uint32_t pipe_id) { int i; From f23a95e44ab4025b569d410aa71b858bca086b49 Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Tue, 21 Sep 2021 16:51:26 -0700 Subject: [PATCH 08/10] dmic: conditionally include variables conditionally used variables should be conditionally compiled in Signed-off-by: Curtis Malainey --- src/drivers/intel/dmic/dmic_computed.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/drivers/intel/dmic/dmic_computed.c b/src/drivers/intel/dmic/dmic_computed.c index 7d4ffef2a635..6eb911a9b020 100644 --- a/src/drivers/intel/dmic/dmic_computed.c +++ b/src/drivers/intel/dmic/dmic_computed.c @@ -593,7 +593,9 @@ static int configure_registers(struct dai *dai, uint32_t ref; int32_t ci; uint32_t cu; +#if defined(DMIC_IPM_VER1) || defined(DMIC_IPM_VER2) int ipm; +#endif int of0; int of1; int fir_decim; From e93e2d4c1a19c0e84122e90e123eacf3b2dc63fe Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Tue, 21 Sep 2021 16:55:00 -0700 Subject: [PATCH 09/10] style: make delcaration match implementations minor style violation, these should match exactly Signed-off-by: Curtis Malainey --- src/include/sof/ipc/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/sof/ipc/common.h b/src/include/sof/ipc/common.h index d1330e3e70fc..5182b548e08b 100644 --- a/src/include/sof/ipc/common.h +++ b/src/include/sof/ipc/common.h @@ -187,7 +187,7 @@ ipc_cmd_hdr *mailbox_validate(void); * * @param hdr Points to the IPC command header. */ -void ipc_cmd(ipc_cmd_hdr *hdr); +void ipc_cmd(ipc_cmd_hdr *_hdr); /** * \brief IPC message to be processed on other core. From 0722e0876887b348d3e914f800d0d546d7d9850a Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Tue, 21 Sep 2021 16:57:20 -0700 Subject: [PATCH 10/10] ipc: don't shadow functions with variables ipc_free is a function, we should avoid shadowing in the event we tried to use the function here. Signed-off-by: Curtis Malainey --- src/ipc/ipc3/handler.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ipc/ipc3/handler.c b/src/ipc/ipc3/handler.c index a87dd30ef9d7..51d747a931b5 100644 --- a/src/ipc/ipc3/handler.c +++ b/src/ipc/ipc3/handler.c @@ -1303,20 +1303,20 @@ static int ipc_glb_tplg_free(uint32_t header, int (*free_func)(struct ipc *ipc, uint32_t id)) { struct ipc *ipc = ipc_get(); - struct sof_ipc_free ipc_free; + struct sof_ipc_free ipc_free_msg; int ret; /* copy message with ABI safe method */ - IPC_COPY_CMD(ipc_free, ipc->comp_data); + IPC_COPY_CMD(ipc_free_msg, ipc->comp_data); - tr_info(&ipc_tr, "ipc: comp %d -> free", ipc_free.id); + tr_info(&ipc_tr, "ipc: comp %d -> free", ipc_free_msg.id); /* free the object */ - ret = free_func(ipc, ipc_free.id); + ret = free_func(ipc, ipc_free_msg.id); if (ret < 0) { tr_err(&ipc_tr, "ipc: comp %d free failed %d", - ipc_free.id, ret); + ipc_free_msg.id, ret); } return ret;