Messages in this thread Patch in this message | | | Date | Fri, 11 Sep 2009 17:05:00 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 3/3] devmem: cleanup unxlate_dev_mem_ptr() calls |
| |
On Fri, 11 Sep 2009 10:23:36 +0800 Wu Fengguang <fengguang.wu@intel.com> wrote:
> No behavior change. > > CC: Marcelo Tosatti <mtosatti@redhat.com> > CC: Greg Kroah-Hartman <gregkh@suse.de> > CC: Mark Brown <broonie@opensource.wolfsonmicro.com> > CC: Johannes Berg <johannes@sipsolutions.net> > CC: Avi Kivity <avi@qumranet.com> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> > --- > drivers/char/mem.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > --- linux-mm.orig/drivers/char/mem.c 2009-09-10 21:59:39.000000000 +0800 > +++ linux-mm/drivers/char/mem.c 2009-09-10 22:00:12.000000000 +0800 > @@ -131,6 +131,7 @@ static ssize_t read_mem(struct file * fi > size_t count, loff_t *ppos) > { > unsigned long p = *ppos; > + unsigned long ret; > ssize_t read, sz; > char *ptr; > > @@ -169,12 +170,10 @@ static ssize_t read_mem(struct file * fi > if (!ptr) > return -EFAULT; > > - if (copy_to_user(buf, ptr, sz)) { > - unxlate_dev_mem_ptr(p, ptr); > - return -EFAULT; > - } > - > + ret = copy_to_user(buf, ptr, sz); > unxlate_dev_mem_ptr(p, ptr); > + if (ret) > + return -EFAULT; > > buf += sz; > p += sz;
- local var `ret' didn't need function-wide scope. I think it's better to reduce its scope if poss.
- conventionally the identifier `ret' refers to "the value which this function will return". Ditto `retval' and `rc'.
But that's not what `ret' does here so let's call it something else? `remaining' is rather verbose and formal, but accurate.
--- a/drivers/char/mem.c~dev-mem-cleanup-unxlate_dev_mem_ptr-calls-fix +++ a/drivers/char/mem.c @@ -131,7 +131,6 @@ static ssize_t read_mem(struct file * fi size_t count, loff_t *ppos) { unsigned long p = *ppos; - unsigned long ret; ssize_t read, sz; char *ptr; @@ -156,6 +155,8 @@ static ssize_t read_mem(struct file * fi #endif while (count > 0) { + unsigned long remaining; + sz = size_inside_page(p, count); if (!range_is_allowed(p >> PAGE_SHIFT, count)) @@ -170,9 +171,9 @@ static ssize_t read_mem(struct file * fi if (!ptr) return -EFAULT; - ret = copy_to_user(buf, ptr, sz); + remaining = copy_to_user(buf, ptr, sz); unxlate_dev_mem_ptr(p, ptr); - if (ret) + if (remaining) return -EFAULT; buf += sz; _
| |