| Message ID | 20190820034331.38171-1-jaska.uimonen@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: SOF: topology: fix get control data return type and arguments | expand |
Added Mark and Takashi in Cc. On 8/19/19 10:43 PM, Jaska Uimonen wrote: > sof_get_control_data returns negative values even though the return > value is defined unsigned (size_t). So change the return value type > to int. > > Add the data size as pointer argument to sof_get_control_data to > avoid ambiquity in the meaning of the return type. Also make the > sof_get_control_data to return -EINVAL if data pointer is valid > but the size is for some reason zero. > > Fixes: cac974a51ebb ("ASoC: SOF: topology: use set_get_data in process load") > Reported by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > --- > sound/soc/sof/topology.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c > index 28a7a6e06a53..8288b1758542 100644 > --- a/sound/soc/sof/topology.c > +++ b/sound/soc/sof/topology.c > @@ -1752,15 +1752,15 @@ static int sof_widget_load_siggen(struct snd_soc_component *scomp, int index, > return ret; > } > > -static size_t sof_get_control_data(struct snd_sof_dev *sdev, > - struct snd_soc_dapm_widget *widget, > - struct sof_widget_data *wdata) > +static int sof_get_control_data(struct snd_sof_dev *sdev, > + struct snd_soc_dapm_widget *widget, > + struct sof_widget_data *wdata, > + size_t *size) > { > const struct snd_kcontrol_new *kc; > struct soc_mixer_control *sm; > struct soc_bytes_ext *sbe; > struct soc_enum *se; > - size_t size = 0; > int i; > > for (i = 0; i < widget->num_kcontrols; i++) { > @@ -1800,7 +1800,11 @@ static size_t sof_get_control_data(struct snd_sof_dev *sdev, > if (wdata[i].pdata->magic != SOF_ABI_MAGIC) > return -EINVAL; > > - size += wdata[i].pdata->size; > + /* don't accept 0 size for data */ > + if (!wdata[i].pdata->size) > + return -EINVAL; > + > + *size += wdata[i].pdata->size; > > /* get data type */ > switch (wdata[i].control->cmd) { > @@ -1819,7 +1823,7 @@ static size_t sof_get_control_data(struct snd_sof_dev *sdev, > } > } > > - return size; > + return 0; > } > > static int sof_process_load(struct snd_soc_component *scomp, int index, > @@ -1855,12 +1859,11 @@ static int sof_process_load(struct snd_soc_component *scomp, int index, > return -ENOMEM; > > /* get possible component controls and get size of all pdata */ > - ipc_data_size = sof_get_control_data(sdev, widget, wdata); > + ret = sof_get_control_data(sdev, widget, wdata, > + &ipc_data_size); > > - if (ipc_data_size <= 0) { > - ret = ipc_data_size; > + if (ret < 0) > goto out; > - } > } > > ipc_size = sizeof(struct sof_ipc_comp_process) + >
On Tue, Aug 20, 2019 at 07:30:25AM -0500, Pierre-Louis Bossart wrote: > Added Mark and Takashi in Cc. > > On 8/19/19 10:43 PM, Jaska Uimonen wrote: > > sof_get_control_data returns negative values even though the return > > value is defined unsigned (size_t). So change the return value type > > to int. Please send me the actual patch, I can't apply quoted material.
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 28a7a6e06a53..8288b1758542 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1752,15 +1752,15 @@ static int sof_widget_load_siggen(struct snd_soc_component *scomp, int index, return ret; } -static size_t sof_get_control_data(struct snd_sof_dev *sdev, - struct snd_soc_dapm_widget *widget, - struct sof_widget_data *wdata) +static int sof_get_control_data(struct snd_sof_dev *sdev, + struct snd_soc_dapm_widget *widget, + struct sof_widget_data *wdata, + size_t *size) { const struct snd_kcontrol_new *kc; struct soc_mixer_control *sm; struct soc_bytes_ext *sbe; struct soc_enum *se; - size_t size = 0; int i; for (i = 0; i < widget->num_kcontrols; i++) { @@ -1800,7 +1800,11 @@ static size_t sof_get_control_data(struct snd_sof_dev *sdev, if (wdata[i].pdata->magic != SOF_ABI_MAGIC) return -EINVAL; - size += wdata[i].pdata->size; + /* don't accept 0 size for data */ + if (!wdata[i].pdata->size) + return -EINVAL; + + *size += wdata[i].pdata->size; /* get data type */ switch (wdata[i].control->cmd) { @@ -1819,7 +1823,7 @@ static size_t sof_get_control_data(struct snd_sof_dev *sdev, } } - return size; + return 0; } static int sof_process_load(struct snd_soc_component *scomp, int index, @@ -1855,12 +1859,11 @@ static int sof_process_load(struct snd_soc_component *scomp, int index, return -ENOMEM; /* get possible component controls and get size of all pdata */ - ipc_data_size = sof_get_control_data(sdev, widget, wdata); + ret = sof_get_control_data(sdev, widget, wdata, + &ipc_data_size); - if (ipc_data_size <= 0) { - ret = ipc_data_size; + if (ret < 0) goto out; - } } ipc_size = sizeof(struct sof_ipc_comp_process) +
sof_get_control_data returns negative values even though the return value is defined unsigned (size_t). So change the return value type to int. Add the data size as pointer argument to sof_get_control_data to avoid ambiquity in the meaning of the return type. Also make the sof_get_control_data to return -EINVAL if data pointer is valid but the size is for some reason zero. Fixes: cac974a51ebb ("ASoC: SOF: topology: use set_get_data in process load") Reported by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com> --- sound/soc/sof/topology.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)