* [PATCH] fs/proc/vmcore.c: Put if tests in the top of the while loop to reduce duplication
@ 2013-02-03 13:13 Zhang Yanfei
2013-02-08 0:06 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Zhang Yanfei @ 2013-02-03 13:13 UTC (permalink / raw)
To: akpm; +Cc: Eric W. Biederman, benh, mahesh, zhangyanfei, linux-kernel
From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
In function read_vmcore, two if tests are duplicate. Change the position
of them could reduce the duplication. This change does not affect
the behaviour of the function.
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
fs/proc/vmcore.c | 21 ++++++++-------------
1 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 0d5071d..c74a901 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -175,15 +175,16 @@ static ssize_t read_vmcore(struct file *file, char __user *buffer,
start = map_offset_to_paddr(*fpos, &vmcore_list, &curr_m);
if (!curr_m)
return -EINVAL;
- if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
- tsz = buflen;
-
- /* Calculate left bytes in current memory segment. */
- nr_bytes = (curr_m->size - (start - curr_m->paddr));
- if (tsz > nr_bytes)
- tsz = nr_bytes;
while (buflen) {
+ if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
+ tsz = buflen;
+
+ /* Calculate left bytes in current memory segment. */
+ nr_bytes = (curr_m->size - (start - curr_m->paddr));
+ if (tsz > nr_bytes)
+ tsz = nr_bytes;
+
tmp = read_from_oldmem(buffer, tsz, &start, 1);
if (tmp < 0)
return tmp;
@@ -198,12 +199,6 @@ static ssize_t read_vmcore(struct file *file, char __user *buffer,
struct vmcore, list);
start = curr_m->paddr;
}
- if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
- tsz = buflen;
- /* Calculate left bytes in current memory segment. */
- nr_bytes = (curr_m->size - (start - curr_m->paddr));
- if (tsz > nr_bytes)
- tsz = nr_bytes;
}
return acc;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fs/proc/vmcore.c: Put if tests in the top of the while loop to reduce duplication
2013-02-03 13:13 [PATCH] fs/proc/vmcore.c: Put if tests in the top of the while loop to reduce duplication Zhang Yanfei
@ 2013-02-08 0:06 ` Andrew Morton
2013-02-08 14:25 ` Eric W. Biederman
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2013-02-08 0:06 UTC (permalink / raw)
To: Zhang Yanfei; +Cc: Eric W. Biederman, benh, mahesh, zhangyanfei, linux-kernel
On Sun, 03 Feb 2013 21:13:36 +0800
Zhang Yanfei <zhangyanfei.yes@gmail.com> wrote:
> From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
>
> In function read_vmcore, two if tests are duplicate. Change the position
> of them could reduce the duplication. This change does not affect
> the behaviour of the function.
>
hm, yes.
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -175,15 +175,16 @@ static ssize_t read_vmcore(struct file *file, char __user *buffer,
> start = map_offset_to_paddr(*fpos, &vmcore_list, &curr_m);
> if (!curr_m)
> return -EINVAL;
> - if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
> - tsz = buflen;
> -
> - /* Calculate left bytes in current memory segment. */
> - nr_bytes = (curr_m->size - (start - curr_m->paddr));
> - if (tsz > nr_bytes)
> - tsz = nr_bytes;
>
> while (buflen) {
> + if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
> + tsz = buflen;
> +
> + /* Calculate left bytes in current memory segment. */
> + nr_bytes = (curr_m->size - (start - curr_m->paddr));
> + if (tsz > nr_bytes)
> + tsz = nr_bytes;
> +
> tmp = read_from_oldmem(buffer, tsz, &start, 1);
> if (tmp < 0)
> return tmp;
> @@ -198,12 +199,6 @@ static ssize_t read_vmcore(struct file *file, char __user *buffer,
> struct vmcore, list);
> start = curr_m->paddr;
`start' gets changed here
> }
> - if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
> - tsz = buflen;
> - /* Calculate left bytes in current memory segment. */
> - nr_bytes = (curr_m->size - (start - curr_m->paddr));
> - if (tsz > nr_bytes)
> - tsz = nr_bytes;
So this code can calcualte a different value of `tsz'. But that won't
get used. Looks right to me.
> }
> return acc;
> }
While we're there, let's squish that assignment-in-a-if pestilence and
use max_t().
--- a/fs/proc/vmcore.c~fs-proc-vmcorec-put-if-tests-in-the-top-of-the-while-loop-to-reduce-duplication-fix
+++ a/fs/proc/vmcore.c
@@ -178,8 +178,7 @@ static ssize_t read_vmcore(struct file *
return -EINVAL;
while (buflen) {
- if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
- tsz = buflen;
+ tsz = max_t(size_t, buflen, PAGE_SIZE - (start & ~PAGE_MASK));
/* Calculate left bytes in current memory segment. */
nr_bytes = (curr_m->size - (start - curr_m->paddr));
_
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs/proc/vmcore.c: Put if tests in the top of the while loop to reduce duplication
2013-02-08 0:06 ` Andrew Morton
@ 2013-02-08 14:25 ` Eric W. Biederman
2013-02-08 17:52 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Eric W. Biederman @ 2013-02-08 14:25 UTC (permalink / raw)
To: Andrew Morton; +Cc: Zhang Yanfei, benh, mahesh, zhangyanfei, linux-kernel
Andrew Morton <akpm@linux-foundation.org> writes:
> On Sun, 03 Feb 2013 21:13:36 +0800
> Zhang Yanfei <zhangyanfei.yes@gmail.com> wrote:
>
>> From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
>>
>> In function read_vmcore, two if tests are duplicate. Change the position
>> of them could reduce the duplication. This change does not affect
>> the behaviour of the function.
> While we're there, let's squish that assignment-in-a-if pestilence and
> use max_t().
min_t please. Or do you mean to change the behavior of the code?
> --- a/fs/proc/vmcore.c~fs-proc-vmcorec-put-if-tests-in-the-top-of-the-while-loop-to-reduce-duplication-fix
> +++ a/fs/proc/vmcore.c
> @@ -178,8 +178,7 @@ static ssize_t read_vmcore(struct file *
> return -EINVAL;
>
> while (buflen) {
> - if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
> - tsz = buflen;
> + tsz = max_t(size_t, buflen, PAGE_SIZE - (start & ~PAGE_MASK));
>
> /* Calculate left bytes in current memory segment. */
> nr_bytes = (curr_m->size - (start - curr_m->paddr));
> _
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs/proc/vmcore.c: Put if tests in the top of the while loop to reduce duplication
2013-02-08 14:25 ` Eric W. Biederman
@ 2013-02-08 17:52 ` Andrew Morton
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2013-02-08 17:52 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Zhang Yanfei, benh, mahesh, zhangyanfei, linux-kernel
On Fri, 08 Feb 2013 06:25:39 -0800 ebiederm@xmission.com (Eric W. Biederman) wrote:
> Andrew Morton <akpm@linux-foundation.org> writes:
>
> > On Sun, 03 Feb 2013 21:13:36 +0800
> > Zhang Yanfei <zhangyanfei.yes@gmail.com> wrote:
> >
> >> From: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
> >>
> >> In function read_vmcore, two if tests are duplicate. Change the position
> >> of them could reduce the duplication. This change does not affect
> >> the behaviour of the function.
>
> > While we're there, let's squish that assignment-in-a-if pestilence and
> > use max_t().
>
> min_t please. Or do you mean to change the behavior of the code?
oops.
> > --- a/fs/proc/vmcore.c~fs-proc-vmcorec-put-if-tests-in-the-top-of-the-while-loop-to-reduce-duplication-fix
> > +++ a/fs/proc/vmcore.c
> > @@ -178,8 +178,7 @@ static ssize_t read_vmcore(struct file *
> > return -EINVAL;
> >
> > while (buflen) {
> > - if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen)
> > - tsz = buflen;
> > + tsz = max_t(size_t, buflen, PAGE_SIZE - (start & ~PAGE_MASK));
> >
> > /* Calculate left bytes in current memory segment. */
> > nr_bytes = (curr_m->size - (start - curr_m->paddr));
> > _
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-02-08 17:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-03 13:13 [PATCH] fs/proc/vmcore.c: Put if tests in the top of the while loop to reduce duplication Zhang Yanfei
2013-02-08 0:06 ` Andrew Morton
2013-02-08 14:25 ` Eric W. Biederman
2013-02-08 17:52 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).