Re: [PATCH] ASoC: soc-core: Support debugfs entries larger than PAGE_SIZE bytes

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Wed, Feb 02, 2011 at 11:29:05AM +0000, Dimitris Papastamos wrote:

> +/* fill buf which is 'len' bytes with a formatted
> + * string of the form 'reg: value\n' */
> +static int format_register_str(struct snd_soc_codec *codec,
> +			       unsigned int regidx, char *buf, size_t len)

The naming of regidx is really inconsistent with the rest of the code
which pretty much always calls registers reg.

> +	/* we don't currently handle failed reads */
> +	ret = snd_soc_read(codec , regidx);
> +	if (ret < 0) {
> +		dev_err(codec->dev, "Failed to read from %#x\n", regidx);
> +		return ret;
> +	}

I'd suggest filling the field with some obvious out of bound character
such as X.

> +	if (ret >= 0) {
> +		if (!access_ok(VERIFY_WRITE, user_buf, ret)) {
> +			ret = -EFAULT;
> +			goto out;
> +		}
> +		if (copy_to_user(user_buf, buf, ret)) {
> +			ret = -EFAULT;
> +			goto out;
> +		}

Why do we need the access_ok() here?  I'd really expect copy_to_user()
to do the right thing here and simple_read_from_buffer() doesn't do
this.