Re: [PATCH 2/2] ASoC: SOF: sof-client-probes: cleanup tokenize_input()

Péter Ujfalusi
2022-07-06T09:28:30Z

On 06/07/2022 10:25, Dan Carpenter wrote:
> The tokenize_input() function is cleaner if it uses strndup_user()
> instead of simple_write_to_buffer(). The way it's written now, if
> *ppos is non-zero then it returns -EIO but normally we would return
> 0 in that case. It's easier to handle that in the callers.

This patch breaks the probe point settings:

# echo 52,1,0 > /sys/kernel/debug/sof/probe_points
-bash: echo: write error: Invalid argument

I did not looked for the exact reason, but something is not correct.

> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> sound/soc/sof/sof-client-probes.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/sound/soc/sof/sof-client-probes.c b/sound/soc/sof/sof-client-probes.c
> index 679bc7d371fc..6c922b683f67 100644
> --- a/sound/soc/sof/sof-client-probes.c
> +++ b/sound/soc/sof/sof-client-probes.c
> @@ -461,24 +461,17 @@ static int strsplit_u32(char *buf, const char *delim, u32 **tkns, size_t *num_tk
> }
>
> static int tokenize_input(const char __user *from, size_t count,
> - loff_t *ppos, u32 **tkns, size_t *num_tkns)
> + u32 **tkns, size_t *num_tkns)
> {
> char *buf;
> int ret;
>
> - buf = kmalloc(count + 1, GFP_KERNEL);
> - if (!buf)
> - return -ENOMEM;
> -
> - ret = simple_write_to_buffer(buf, count, ppos, from, count);
> - if (ret != count) {
> - ret = ret >= 0 ? -EIO : ret;
> - goto exit;
> - }
> + buf = strndup_user(from, count + 1);
> + if (IS_ERR(buf))
> + return PTR_ERR(buf);
>
> - buf[count] = '\0';
> ret = strsplit_u32(buf, ",", tkns, num_tkns);
> -exit:
> +
> kfree(buf);
> return ret;
> }
> @@ -552,12 +545,15 @@ sof_probes_dfs_points_write(struct file *file, const char __user *from,
> u32 *tkns;
> int ret, err;
>
> + if (*ppos)
> + return 0;
> +
> if (priv->extractor_stream_tag == SOF_PROBES_INVALID_NODE_ID) {
> dev_warn(dev, "no extractor stream running\n");
> return -ENOENT;
> }
>
> - ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
> + ret = tokenize_input(from, count, &tkns, &num_tkns);
> if (ret < 0)
> return ret;
> bytes = sizeof(*tkns) * num_tkns;
> @@ -607,12 +603,15 @@ sof_probes_dfs_points_remove_write(struct file *file, const char __user *from,
> u32 *tkns;
> int ret, err;
>
> + if (*ppos)
> + return 0;
> +
> if (priv->extractor_stream_tag == SOF_PROBES_INVALID_NODE_ID) {
> dev_warn(dev, "no extractor stream running\n");
> return -ENOENT;
> }
>
> - ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
> + ret = tokenize_input(from, count, &tkns, &num_tkns);
> if (ret < 0)
> return ret;
> if (!num_tkns) {

--
Péter