7 Aug
2019
7 Aug
'19
10:17 p.m.
On 8/7/19 3:09 PM, Sridharan, Ranjani wrote:
On Wed, Aug 7, 2019 at 12:32 PM Cezary Rojewski <cezary.rojewski@intel.com mailto:cezary.rojewski@intel.com> wrote:
On 2019-08-07 16:52, Pierre-Louis Bossart wrote: > From: Jaska Uimonen <jaska.uimonen@intel.com <mailto:jaska.uimonen@intel.com>> > process = kzalloc(ipc_size, GFP_KERNEL); > - if (!process) > + if (!process) { > + kfree(wdata); > return -ENOMEM; > + } > > /* configure iir IPC message */ > process->comp.hdr.size = ipc_size; > @@ -1835,7 +1890,9 @@ static int sof_process_load(struct snd_soc_component *scomp, int index, > if (ret != 0) { > dev_err(sdev->dev, "error: parse process.cfg tokens failed %d\n", > le32_to_cpu(private->size)); > - goto err; > + kfree(wdata); > + kfree(process); > + return ret; > } > > @@ -1886,10 +1916,36 @@ static int sof_process_load(struct snd_soc_component *scomp, int index, > > ret = sof_ipc_tx_message(sdev->ipc, process->comp.hdr.cmd, process, > ipc_size, r, sizeof(*r)); > - if (ret >= 0) > + > + if (ret < 0) { > + dev_err(sdev->dev, "error: create process failed\n"); > + kfree(wdata); > + kfree(process); > return ret; > -err: > - kfree(process); > + } > + > + /* we sent the data in single message so return */ > + if (ipc_data_size) { > + kfree(wdata); > + return ret; > + } > + > + /* send control data with large message supported method */ > + for (i = 0; i < widget->num_kcontrols; i++) { > + wdata[i].control->readback_offset = 0; > + ret = snd_sof_ipc_set_get_comp_data(sdev->ipc, wdata[i].control, > + wdata[i].ipc_cmd, > + wdata[i].ctrl_type, > + wdata[i].control->cmd, > + true); > + if (ret != 0) { > + dev_err(sdev->dev, "error: send control failed\n"); > + kfree(process); > + break; > + } > + } > + > + kfree(wdata); > return ret; > } On several occasions you've added individual error paths instead of a unified one. Personally I don't find it easier to read and understand function's flow at all. <ifs with goto err> err: kfree(process); kfree(wdata); return ret; doesn't look that bad..Thanks for pointing out. Perhaps, the error handling can be improved a little. We can fix in v2.
I took a look at this and there's really only 2/3 places where we could use a goto, but we'd have to use 2 labels depending on whether we free process/wdata so not sure if we'd make the code more self-explanatory in the end.
Jaska, can you take a look?