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

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

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