Skip to content

Cppcheck fixes #4818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 9 commits into from
Sep 30, 2021
Merged

Cppcheck fixes #4818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0ab79fe
ipc4: copier: fix return value on comp_trigger
plbossart Sep 28, 2021
82d039c
ipc4: dai: fix NULL pointer dereference in error message
plbossart Sep 28, 2021
5e9ad77
ipc4: dai: fix return values
plbossart Sep 28, 2021
f130e36
ipc3: dai: avoid variable/function shadowing
plbossart Sep 28, 2021
2018ad7
pipeline: simplify handling of first pipeline
plbossart Sep 28, 2021
59f8629
smart_amp: fix error handling
plbossart Sep 28, 2021
e4a420e
mediatek: mt8195: afe-drv: remove redundant initialization
plbossart Sep 28, 2021
edc4587
amd: renoir: clk: fix infinite loop
plbossart Sep 28, 2021
45d97cb
amd: renoir: add error check
plbossart Sep 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/audio/copier.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ static int copier_comp_trigger(struct comp_dev *dev, int cmd)
if (cd->endpoint)
ret = cd->endpoint->drv->ops.trigger(cd->endpoint, cmd);

return 0;
return ret;
}

/* copy and process stream data from source to sink buffers */
Expand Down
5 changes: 3 additions & 2 deletions src/audio/pipeline/pipeline-schedule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx,
struct list_item *tlist;
struct pipeline *p;
uint32_t flags;
bool first_pipe = false;

/*
* Interrupts have to be disabled while adding tasks to or removing them
Expand All @@ -180,10 +181,10 @@ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx,
p = container_of(tlist, struct pipeline, list);
if (pipeline_is_timer_driven(p)) {
/* Use the first of connected pipelines to trigger */
if (cmd >= 0) {
if (cmd >= 0 && !first_pipe) {
Copy link
Copy Markdown

@bhiregoudar bhiregoudar Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the cmd is being checked for >= 0 here? control won't come here if the switch case doesn't match

Copy link
Copy Markdown
Member Author

@plbossart plbossart Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

humm, yes, maybe we only need the !first _pipe. @lyakh would you concur?

Copy link
Copy Markdown
Collaborator

@lyakh lyakh Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually don't find this "comment by cppcheck" very useful... yes, cmd is used as a local variable here, it absolutely isn't expected to have any effect outside of this function. But ok, using a separate variable makes this more explicit. But otherwise - yes, after this change the cmd >= 0 test can be removed there. But first_pipe also seems inverted to me: it is false while the loop is handling the first pipeline and it becomes true for the second and any further pipelines. Shouldn't this be inverted?

Copy link
Copy Markdown
Member Author

@plbossart plbossart Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I still don't know my left from my right... will fix, thanks @lyakh

Copy link
Copy Markdown
Member Author

@plbossart plbossart Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merged too quickly @lgirdwood, i'll send a follow-up to fix this.

Copy link
Copy Markdown
Member Author

@plbossart plbossart Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix in #4832

p->trigger.cmd = cmd;
p->trigger.host = ppl_data->start;
cmd = -EINVAL;
first_pipe = true;
}
} else {
p->xrun_bytes = 0;
Expand Down
6 changes: 2 additions & 4 deletions src/audio/smart_amp/smart_amp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,6 @@ static struct comp_dev *smart_amp_new(const struct comp_driver *drv,

if (bs > 0 && bs < sizeof(struct sof_smart_amp_config)) {
comp_err(dev, "smart_amp_new(): failed to apply config");

if (sad)
rfree(sad);
rfree(sad);
goto error;
}

Expand Down Expand Up @@ -282,6 +278,8 @@ static struct comp_dev *smart_amp_new(const struct comp_driver *drv,
return dev;

error:
rfree(sad);
rfree(dev);
return NULL;
}

Expand Down
4 changes: 2 additions & 2 deletions src/drivers/mediatek/mt8195/afe-drv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ unsigned int afe_memif_get_cur_position(struct mtk_base_afe *afe, int id)
int afe_dai_set_config(struct mtk_base_afe *afe, int id, unsigned int channel, unsigned int rate,
unsigned int format)
{
struct mtk_base_afe_dai *dai = &afe->dais[id];
struct mtk_base_afe_dai *dai;

/* TODO 1. if need use dai->id to search target dai */
/* TODO 1. if need a status to control the dai status */
Expand All @@ -256,7 +256,7 @@ int afe_dai_set_config(struct mtk_base_afe *afe, int id, unsigned int channel, u
int afe_dai_get_config(struct mtk_base_afe *afe, int id, unsigned int *channel, unsigned int *rate,
unsigned int *format)
{
struct mtk_base_afe_dai *dai = &afe->dais[id];
struct mtk_base_afe_dai *dai;

/* TODO 1. if need use dai->id to search target dai */
/* TODO 1. if need a status to control the dai status */
Expand Down
16 changes: 8 additions & 8 deletions src/ipc/ipc3/dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ int ipc_dai_data_config(struct comp_dev *dev)
{
struct dai_data *dd = comp_get_drvdata(dev);
struct ipc_config_dai *dai = &dd->ipc_config;
struct sof_ipc_dai_config *dai_config = ipc_from_dai_config(dd->dai_spec_config);
struct sof_ipc_dai_config *config = ipc_from_dai_config(dd->dai_spec_config);

if (!dai_config) {
if (!config) {
comp_err(dev, "dai_data_config(): no config set for dai %d type %d",
dai->dai_index, dai->type);
return -EINVAL;
Expand All @@ -115,18 +115,18 @@ int ipc_dai_data_config(struct comp_dev *dev)
return -EINVAL;
}

switch (dai_config->type) {
switch (config->type) {
case SOF_DAI_INTEL_SSP:
/* set dma burst elems to slot number */
dd->config.burst_elems = dai_config->ssp.tdm_slots;
dd->config.burst_elems = config->ssp.tdm_slots;
break;
case SOF_DAI_INTEL_DMIC:
/* We can use always the largest burst length. */
dd->config.burst_elems = 8;

comp_info(dev, "config->dmic.fifo_bits = %u config->dmic.num_pdm_active = %u",
dai_config->dmic.fifo_bits,
dai_config->dmic.num_pdm_active);
config->dmic.fifo_bits,
config->dmic.num_pdm_active);
break;
case SOF_DAI_INTEL_HDA:
break;
Expand All @@ -143,7 +143,7 @@ int ipc_dai_data_config(struct comp_dev *dev)
/* As with HDA, the DMA channel is assigned in runtime,
* not during topology parsing.
*/
dd->stream_id = dai_config->alh.stream_id;
dd->stream_id = config->alh.stream_id;
break;
case SOF_DAI_IMX_SAI:
COMPILER_FALLTHROUGH;
Expand All @@ -166,7 +166,7 @@ int ipc_dai_data_config(struct comp_dev *dev)
default:
/* other types of DAIs not handled for now */
comp_warn(dev, "dai_data_config(): Unknown dai type %d",
dai_config->type);
config->type);
break;
}

Expand Down
6 changes: 3 additions & 3 deletions src/ipc/ipc4/dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ int ipc_dai_data_config(struct comp_dev *dev)
struct alh_pdata *alh;

if (!dai) {
comp_err(dev, "dai_data_config(): no config set for dai %d type %d",
dai->dai_index, dai->type);
comp_err(dev, "dai_data_config(): no dai!\n");
return -EINVAL;
}

Expand Down Expand Up @@ -139,7 +138,7 @@ int dai_config(struct comp_dev *dev, struct ipc_config_dai *common_config,
struct ipc4_copier_module_cfg *copier_cfg = spec_config;
struct dai_data *dd = comp_get_drvdata(dev);
int size;
int ret = 0;
int ret;

/* ignore if message not for this DAI id/type */
if (dd->ipc_config.dai_index != common_config->dai_index ||
Expand Down Expand Up @@ -185,6 +184,7 @@ int dai_config(struct comp_dev *dev, struct ipc_config_dai *common_config,
if (ret < 0) {
rfree(dd->dai_spec_config);
dd->dai_spec_config = NULL;
return -EINVAL;
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/platform/amd/renoir/lib/clk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ static void acp_reg_write_via_smn(uint32_t reg_offset,
ACP_SRBM_CYCLE_STS);
if (!acp_srbm_cycle_sts.bits.srbm_clients_sts)
return;
} delay_cnt--;
delay_cnt--;
}
}

static void get_response_from_smu(void)
Expand Down
6 changes: 4 additions & 2 deletions src/platform/amd/renoir/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,15 @@ int platform_init(struct sof *sof)
clock_set_freq(CLK_CPU(cpu_get_id()), CLK_MAX_CPU_HZ);
/* init DMA */
ret = acp_dma_init(sof);
if (ret < 0)
return -ENODEV;
/* Init DMA platform domain */
sof->platform_dma_domain = dma_multi_chan_domain_init(&sof->dma_info->dma_array[0],
sizeof(sof->dma_info->dma_array), PLATFORM_DEFAULT_CLOCK, true);
scheduler_init_ll(sof->platform_dma_domain);
/* initialize the host IPC mechanims */
/* initialize the host IPC mechanisms */
ipc_init(sof);
/* initialize the DAI mechanims */
/* initialize the DAI mechanisms */
ret = dai_init(sof);
if (ret < 0)
return -ENODEV;
Expand Down