public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: peng.fan@nxp.com
Cc: gregkh@linuxfoundation.org, rafael@kernel.org, linux-imx@nxp.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regmap: debugfs: check count when read regmap file
Date: Mon, 9 Mar 2020 12:05:42 +0000	[thread overview]
Message-ID: <20200309120542.GC4101@sirena.org.uk> (raw)
In-Reply-To: <1583673058-20531-1-git-send-email-peng.fan@nxp.com>

[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]

On Sun, Mar 08, 2020 at 09:10:58PM +0800, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>

> @@ -283,9 +283,10 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
>  				    size_t count, loff_t *ppos)
>  {
>  	struct regmap *map = file->private_data;
> +	size_t num = count > map->max_register ? map->max_register : count;

I can see that it might be useful to limit the read size (though our
error checking is doing the right thing here, it's just that kmalloc()
is very verbose) but this doesn't seem like a good limit, especially for
smaller register maps.  Since it's limiting reads to the number of
registers it's going to result in it being impossible to dump the full
register map in a single read.  This is fine from a filesystem API point
of view, reads can always return less data than was asked for, but it's
annoying from the point of view of anyone hacking together something
like a little program to monitor a specific register during testing or
whatever.  If the register map is small enough you won't even be able to
read a single register in a read which is going to be annoying.  Having
either a lower bound or a more generous upper bound would be better.

Please also write normal conditional statements, the ternery operator
isn't great for legibility.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

      parent reply	other threads:[~2020-03-09 12:05 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-08 13:10 [PATCH] regmap: debugfs: check count when read regmap file peng.fan
2020-03-09 11:40 ` Mark Brown
2020-03-09 12:05 ` Mark Brown [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200309120542.GC4101@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peng.fan@nxp.com \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox