* [PATCH v3 1/2] kexec: fix wrong types of some local variables
@ 2013-02-26 5:15 ` Zhang Yanfei
0 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 5:15 UTC (permalink / raw)
To: Andrew Morton, Eric W. Biederman, Simon Horman; +Cc: kexec, linux-kernel
The types of the following local variables:
- ubytes/mbytes in kimage_load_crash_segment()/kimage_load_normal_segment()
- r in vmcoreinfo_append_str()
are wrong, so fix them.
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
kernel/kexec.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 2436ffc..3cbfcc7 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -789,7 +789,7 @@ static int kimage_load_normal_segment(struct kimage *image,
struct kexec_segment *segment)
{
unsigned long maddr;
- unsigned long ubytes, mbytes;
+ size_t ubytes, mbytes;
int result;
unsigned char __user *buf;
@@ -853,7 +853,7 @@ static int kimage_load_crash_segment(struct kimage *image,
* We do things a page at a time for the sake of kmap.
*/
unsigned long maddr;
- unsigned long ubytes, mbytes;
+ size_t ubytes, mbytes;
int result;
unsigned char __user *buf;
@@ -1455,7 +1455,7 @@ void vmcoreinfo_append_str(const char *fmt, ...)
{
va_list args;
char buf[0x50];
- int r;
+ size_t r;
va_start(args, fmt);
r = vsnprintf(buf, sizeof(buf), fmt, args);
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 1/2] kexec: fix wrong types of some local variables
@ 2013-02-26 5:15 ` Zhang Yanfei
0 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 5:15 UTC (permalink / raw)
To: Andrew Morton, Eric W. Biederman, Simon Horman; +Cc: kexec, linux-kernel
The types of the following local variables:
- ubytes/mbytes in kimage_load_crash_segment()/kimage_load_normal_segment()
- r in vmcoreinfo_append_str()
are wrong, so fix them.
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
kernel/kexec.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 2436ffc..3cbfcc7 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -789,7 +789,7 @@ static int kimage_load_normal_segment(struct kimage *image,
struct kexec_segment *segment)
{
unsigned long maddr;
- unsigned long ubytes, mbytes;
+ size_t ubytes, mbytes;
int result;
unsigned char __user *buf;
@@ -853,7 +853,7 @@ static int kimage_load_crash_segment(struct kimage *image,
* We do things a page at a time for the sake of kmap.
*/
unsigned long maddr;
- unsigned long ubytes, mbytes;
+ size_t ubytes, mbytes;
int result;
unsigned char __user *buf;
@@ -1455,7 +1455,7 @@ void vmcoreinfo_append_str(const char *fmt, ...)
{
va_list args;
char buf[0x50];
- int r;
+ size_t r;
va_start(args, fmt);
r = vsnprintf(buf, sizeof(buf), fmt, args);
--
1.7.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/2] kexec: Use min_t to simplify logic
2013-02-26 5:15 ` Zhang Yanfei
@ 2013-02-26 5:30 ` Zhang Yanfei
-1 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 5:30 UTC (permalink / raw)
To: Andrew Morton, Eric W. Biederman, Simon Horman; +Cc: kexec, linux-kernel
This is just a tweak: using min_t to simplify logic of variable
assignments.
v3:
- cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.
v2:
- Rewrite patch description as Simon suggested.
- Fix an inappropriate if test introduced by v1. Thanks Simon.
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
kernel/kexec.c | 24 ++++++++----------------
1 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 3cbfcc7..b6b16f9 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
/* Start with a clear page */
clear_page(ptr);
ptr += maddr & ~PAGE_MASK;
- mchunk = PAGE_SIZE - (maddr & ~PAGE_MASK);
- if (mchunk > mbytes)
- mchunk = mbytes;
-
- uchunk = mchunk;
- if (uchunk > ubytes)
- uchunk = ubytes;
+ mchunk = min_t(size_t, mbytes,
+ (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
+ uchunk = min_t(size_t, ubytes, mchunk);
result = copy_from_user(ptr, buf, uchunk);
kunmap(page);
@@ -874,13 +870,10 @@ static int kimage_load_crash_segment(struct kimage *image,
}
ptr = kmap(page);
ptr += maddr & ~PAGE_MASK;
- mchunk = PAGE_SIZE - (maddr & ~PAGE_MASK);
- if (mchunk > mbytes)
- mchunk = mbytes;
-
- uchunk = mchunk;
- if (uchunk > ubytes) {
- uchunk = ubytes;
+ mchunk = min_t(size_t, mbytes,
+ (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
+ uchunk = min_t(size_t, ubytes, mchunk);
+ if (mchunk > uchunk) {
/* Zero the trailing part of the page */
memset(ptr + uchunk, 0, mchunk - uchunk);
}
@@ -1461,8 +1454,7 @@ void vmcoreinfo_append_str(const char *fmt, ...)
r = vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
- if (r + vmcoreinfo_size > vmcoreinfo_max_size)
- r = vmcoreinfo_max_size - vmcoreinfo_size;
+ r = min_t(size_t, r, vmcoreinfo_max_size - vmcoreinfo_size);
memcpy(&vmcoreinfo_data[vmcoreinfo_size], buf, r);
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/2] kexec: Use min_t to simplify logic
@ 2013-02-26 5:30 ` Zhang Yanfei
0 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 5:30 UTC (permalink / raw)
To: Andrew Morton, Eric W. Biederman, Simon Horman; +Cc: kexec, linux-kernel
This is just a tweak: using min_t to simplify logic of variable
assignments.
v3:
- cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.
v2:
- Rewrite patch description as Simon suggested.
- Fix an inappropriate if test introduced by v1. Thanks Simon.
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
kernel/kexec.c | 24 ++++++++----------------
1 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 3cbfcc7..b6b16f9 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
/* Start with a clear page */
clear_page(ptr);
ptr += maddr & ~PAGE_MASK;
- mchunk = PAGE_SIZE - (maddr & ~PAGE_MASK);
- if (mchunk > mbytes)
- mchunk = mbytes;
-
- uchunk = mchunk;
- if (uchunk > ubytes)
- uchunk = ubytes;
+ mchunk = min_t(size_t, mbytes,
+ (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
+ uchunk = min_t(size_t, ubytes, mchunk);
result = copy_from_user(ptr, buf, uchunk);
kunmap(page);
@@ -874,13 +870,10 @@ static int kimage_load_crash_segment(struct kimage *image,
}
ptr = kmap(page);
ptr += maddr & ~PAGE_MASK;
- mchunk = PAGE_SIZE - (maddr & ~PAGE_MASK);
- if (mchunk > mbytes)
- mchunk = mbytes;
-
- uchunk = mchunk;
- if (uchunk > ubytes) {
- uchunk = ubytes;
+ mchunk = min_t(size_t, mbytes,
+ (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
+ uchunk = min_t(size_t, ubytes, mchunk);
+ if (mchunk > uchunk) {
/* Zero the trailing part of the page */
memset(ptr + uchunk, 0, mchunk - uchunk);
}
@@ -1461,8 +1454,7 @@ void vmcoreinfo_append_str(const char *fmt, ...)
r = vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
- if (r + vmcoreinfo_size > vmcoreinfo_max_size)
- r = vmcoreinfo_max_size - vmcoreinfo_size;
+ r = min_t(size_t, r, vmcoreinfo_max_size - vmcoreinfo_size);
memcpy(&vmcoreinfo_data[vmcoreinfo_size], buf, r);
--
1.7.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
2013-02-26 5:30 ` Zhang Yanfei
@ 2013-02-26 8:38 ` Joe Perches
-1 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2013-02-26 8:38 UTC (permalink / raw)
To: Zhang Yanfei
Cc: Andrew Morton, Eric W. Biederman, Simon Horman, kexec, linux-kernel
On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
> This is just a tweak: using min_t to simplify logic of variable
> assignments.
>
> v3:
> - cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.
Why? Isn't this just a redundant cast?
> diff --git a/kernel/kexec.c b/kernel/kexec.c
[]
> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
[]
> + mchunk = min_t(size_t, mbytes,
> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
#define min_t(type, x, y) ({ \
type __min1 = (x); \
type __min2 = (y); \
__min1 < __min2 ? __min1: __min2; })
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
@ 2013-02-26 8:38 ` Joe Perches
0 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2013-02-26 8:38 UTC (permalink / raw)
To: Zhang Yanfei
Cc: Andrew Morton, Simon Horman, kexec, Eric W. Biederman, linux-kernel
On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
> This is just a tweak: using min_t to simplify logic of variable
> assignments.
>
> v3:
> - cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.
Why? Isn't this just a redundant cast?
> diff --git a/kernel/kexec.c b/kernel/kexec.c
[]
> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
[]
> + mchunk = min_t(size_t, mbytes,
> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
#define min_t(type, x, y) ({ \
type __min1 = (x); \
type __min2 = (y); \
__min1 < __min2 ? __min1: __min2; })
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
2013-02-26 8:38 ` Joe Perches
@ 2013-02-26 8:49 ` Zhang Yanfei
-1 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 8:49 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Eric W. Biederman, Simon Horman, kexec, linux-kernel
于 2013年02月26日 16:38, Joe Perches 写道:
> On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
>> This is just a tweak: using min_t to simplify logic of variable
>> assignments.
>>
>> v3:
>> - cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.
>
> Why? Isn't this just a redundant cast?
>
>> diff --git a/kernel/kexec.c b/kernel/kexec.c
> []
>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
> []
>> + mchunk = min_t(size_t, mbytes,
>> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
>
> #define min_t(type, x, y) ({ \
> type __min1 = (x); \
> type __min2 = (y); \
> __min1 < __min2 ? __min1: __min2; })
>
>
>
Hmm, from the definition, the cast is redundant.
Maybe I misunderstood what Andrew meant in the mail related to v2:
"The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
different types on different architectures, so some form of casting is
unavoidable here."
Andrew, could you please explain the casting you meant above?
Thanks
Zhang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
@ 2013-02-26 8:49 ` Zhang Yanfei
0 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 8:49 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Simon Horman, kexec, Eric W. Biederman, linux-kernel
于 2013年02月26日 16:38, Joe Perches 写道:
> On Tue, 2013-02-26 at 13:30 +0800, Zhang Yanfei wrote:
>> This is just a tweak: using min_t to simplify logic of variable
>> assignments.
>>
>> v3:
>> - cast type of (PAGE_SIZE - (maddr & ~PAGE_MASK)) into size_t.
>
> Why? Isn't this just a redundant cast?
>
>> diff --git a/kernel/kexec.c b/kernel/kexec.c
> []
>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
> []
>> + mchunk = min_t(size_t, mbytes,
>> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
>
> #define min_t(type, x, y) ({ \
> type __min1 = (x); \
> type __min2 = (y); \
> __min1 < __min2 ? __min1: __min2; })
>
>
>
Hmm, from the definition, the cast is redundant.
Maybe I misunderstood what Andrew meant in the mail related to v2:
"The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
different types on different architectures, so some form of casting is
unavoidable here."
Andrew, could you please explain the casting you meant above?
Thanks
Zhang
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
2013-02-26 8:49 ` Zhang Yanfei
@ 2013-02-26 8:53 ` Andrew Morton
-1 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2013-02-26 8:53 UTC (permalink / raw)
To: Zhang Yanfei
Cc: Joe Perches, Eric W. Biederman, Simon Horman, kexec, linux-kernel
On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
> >> diff --git a/kernel/kexec.c b/kernel/kexec.c
> > []
> >> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
> > []
> >> + mchunk = min_t(size_t, mbytes,
> >> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
> >
> > #define min_t(type, x, y) ({ \
> > type __min1 = (x); \
> > type __min2 = (y); \
> > __min1 < __min2 ? __min1: __min2; })
> >
> >
> >
>
> Hmm, from the definition, the cast is redundant.
>
> Maybe I misunderstood what Andrew meant in the mail related to v2:
>
> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
> different types on different architectures, so some form of casting is
> unavoidable here."
>
> Andrew, could you please explain the casting you meant above?
I mean that a cast (or min_t, which is a cast) will be needed.
The code you have here casts the same thing two times, which isn't
necessary.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
@ 2013-02-26 8:53 ` Andrew Morton
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2013-02-26 8:53 UTC (permalink / raw)
To: Zhang Yanfei
Cc: Joe Perches, Simon Horman, kexec, Eric W. Biederman, linux-kernel
On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
> >> diff --git a/kernel/kexec.c b/kernel/kexec.c
> > []
> >> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
> > []
> >> + mchunk = min_t(size_t, mbytes,
> >> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
> >
> > #define min_t(type, x, y) ({ \
> > type __min1 = (x); \
> > type __min2 = (y); \
> > __min1 < __min2 ? __min1: __min2; })
> >
> >
> >
>
> Hmm, from the definition, the cast is redundant.
>
> Maybe I misunderstood what Andrew meant in the mail related to v2:
>
> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
> different types on different architectures, so some form of casting is
> unavoidable here."
>
> Andrew, could you please explain the casting you meant above?
I mean that a cast (or min_t, which is a cast) will be needed.
The code you have here casts the same thing two times, which isn't
necessary.
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
2013-02-26 8:53 ` Andrew Morton
@ 2013-02-26 8:55 ` Zhang Yanfei
-1 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 8:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Joe Perches, Eric W. Biederman, Simon Horman, kexec, linux-kernel
于 2013年02月26日 16:53, Andrew Morton 写道:
> On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
>
>>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>>> []
>>>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
>>> []
>>>> + mchunk = min_t(size_t, mbytes,
>>>> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
>>>
>>> #define min_t(type, x, y) ({ \
>>> type __min1 = (x); \
>>> type __min2 = (y); \
>>> __min1 < __min2 ? __min1: __min2; })
>>>
>>>
>>>
>>
>> Hmm, from the definition, the cast is redundant.
>>
>> Maybe I misunderstood what Andrew meant in the mail related to v2:
>>
>> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
>> different types on different architectures, so some form of casting is
>> unavoidable here."
>>
>> Andrew, could you please explain the casting you meant above?
>
> I mean that a cast (or min_t, which is a cast) will be needed.
> The code you have here casts the same thing two times, which isn't
> necessary.
>
>
Thanks.
Should I resend the patch again? After removing this redundant cast,
it is the same with v2.
Zhang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
@ 2013-02-26 8:55 ` Zhang Yanfei
0 siblings, 0 replies; 16+ messages in thread
From: Zhang Yanfei @ 2013-02-26 8:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Joe Perches, Simon Horman, kexec, Eric W. Biederman, linux-kernel
于 2013年02月26日 16:53, Andrew Morton 写道:
> On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
>
>>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>>> []
>>>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
>>> []
>>>> + mchunk = min_t(size_t, mbytes,
>>>> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
>>>
>>> #define min_t(type, x, y) ({ \
>>> type __min1 = (x); \
>>> type __min2 = (y); \
>>> __min1 < __min2 ? __min1: __min2; })
>>>
>>>
>>>
>>
>> Hmm, from the definition, the cast is redundant.
>>
>> Maybe I misunderstood what Andrew meant in the mail related to v2:
>>
>> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
>> different types on different architectures, so some form of casting is
>> unavoidable here."
>>
>> Andrew, could you please explain the casting you meant above?
>
> I mean that a cast (or min_t, which is a cast) will be needed.
> The code you have here casts the same thing two times, which isn't
> necessary.
>
>
Thanks.
Should I resend the patch again? After removing this redundant cast,
it is the same with v2.
Zhang
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
2013-02-26 8:55 ` Zhang Yanfei
@ 2013-02-27 22:31 ` Andrew Morton
-1 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2013-02-27 22:31 UTC (permalink / raw)
To: Zhang Yanfei
Cc: Joe Perches, Eric W. Biederman, Simon Horman, kexec, linux-kernel
On Tue, 26 Feb 2013 16:55:57 +0800
Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
> ___ 2013___02___26___ 16:53, Andrew Morton ______:
> > On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
> >
> >>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
> >>> []
> >>>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
> >>> []
> >>>> + mchunk = min_t(size_t, mbytes,
> >>>> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
> >>>
> >>> #define min_t(type, x, y) ({ \
> >>> type __min1 = (x); \
> >>> type __min2 = (y); \
> >>> __min1 < __min2 ? __min1: __min2; })
> >>>
> >>>
> >>>
> >>
> >> Hmm, from the definition, the cast is redundant.
> >>
> >> Maybe I misunderstood what Andrew meant in the mail related to v2:
> >>
> >> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
> >> different types on different architectures, so some form of casting is
> >> unavoidable here."
> >>
> >> Andrew, could you please explain the casting you meant above?
> >
> > I mean that a cast (or min_t, which is a cast) will be needed.
> > The code you have here casts the same thing two times, which isn't
> > necessary.
> >
> >
>
> Thanks.
>
> Should I resend the patch again? After removing this redundant cast,
> it is the same with v2.
That patch was pretty busted. This should fix everything up:
--- a/kernel/kexec.c~kexec-use-min_t-to-simplify-logic-fix
+++ a/kernel/kexec.c
@@ -820,8 +820,8 @@ static int kimage_load_normal_segment(st
clear_page(ptr);
ptr += maddr & ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
- (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
- uchunk = min_t(size_t, ubytes, mchunk);
+ PAGE_SIZE - (maddr & ~PAGE_MASK));
+ uchunk = min(ubytes, mchunk);
result = copy_from_user(ptr, buf, uchunk);
kunmap(page);
@@ -868,8 +868,8 @@ static int kimage_load_crash_segment(str
ptr = kmap(page);
ptr += maddr & ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
- (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
- uchunk = min_t(size_t, ubytes, mchunk);
+ PAGE_SIZE - (maddr & ~PAGE_MASK));
+ uchunk = min(ubytes, mchunk);
if (mchunk > uchunk) {
/* Zero the trailing part of the page */
memset(ptr + uchunk, 0, mchunk - uchunk);
@@ -1451,7 +1451,7 @@ void vmcoreinfo_append_str(const char *f
r = vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
- r = min_t(size_t, r, vmcoreinfo_max_size - vmcoreinfo_size);
+ r = min(r, vmcoreinfo_max_size - vmcoreinfo_size);
memcpy(&vmcoreinfo_data[vmcoreinfo_size], buf, r);
_
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic
@ 2013-02-27 22:31 ` Andrew Morton
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2013-02-27 22:31 UTC (permalink / raw)
To: Zhang Yanfei
Cc: Joe Perches, Simon Horman, kexec, Eric W. Biederman, linux-kernel
On Tue, 26 Feb 2013 16:55:57 +0800
Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
> ___ 2013___02___26___ 16:53, Andrew Morton ______:
> > On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@cn.fujitsu.com> wrote:
> >
> >>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
> >>> []
> >>>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
> >>> []
> >>>> + mchunk = min_t(size_t, mbytes,
> >>>> + (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
> >>>
> >>> #define min_t(type, x, y) ({ \
> >>> type __min1 = (x); \
> >>> type __min2 = (y); \
> >>> __min1 < __min2 ? __min1: __min2; })
> >>>
> >>>
> >>>
> >>
> >> Hmm, from the definition, the cast is redundant.
> >>
> >> Maybe I misunderstood what Andrew meant in the mail related to v2:
> >>
> >> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
> >> different types on different architectures, so some form of casting is
> >> unavoidable here."
> >>
> >> Andrew, could you please explain the casting you meant above?
> >
> > I mean that a cast (or min_t, which is a cast) will be needed.
> > The code you have here casts the same thing two times, which isn't
> > necessary.
> >
> >
>
> Thanks.
>
> Should I resend the patch again? After removing this redundant cast,
> it is the same with v2.
That patch was pretty busted. This should fix everything up:
--- a/kernel/kexec.c~kexec-use-min_t-to-simplify-logic-fix
+++ a/kernel/kexec.c
@@ -820,8 +820,8 @@ static int kimage_load_normal_segment(st
clear_page(ptr);
ptr += maddr & ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
- (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
- uchunk = min_t(size_t, ubytes, mchunk);
+ PAGE_SIZE - (maddr & ~PAGE_MASK));
+ uchunk = min(ubytes, mchunk);
result = copy_from_user(ptr, buf, uchunk);
kunmap(page);
@@ -868,8 +868,8 @@ static int kimage_load_crash_segment(str
ptr = kmap(page);
ptr += maddr & ~PAGE_MASK;
mchunk = min_t(size_t, mbytes,
- (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
- uchunk = min_t(size_t, ubytes, mchunk);
+ PAGE_SIZE - (maddr & ~PAGE_MASK));
+ uchunk = min(ubytes, mchunk);
if (mchunk > uchunk) {
/* Zero the trailing part of the page */
memset(ptr + uchunk, 0, mchunk - uchunk);
@@ -1451,7 +1451,7 @@ void vmcoreinfo_append_str(const char *f
r = vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
- r = min_t(size_t, r, vmcoreinfo_max_size - vmcoreinfo_size);
+ r = min(r, vmcoreinfo_max_size - vmcoreinfo_size);
memcpy(&vmcoreinfo_data[vmcoreinfo_size], buf, r);
_
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/2] kexec: fix wrong types of some local variables
2013-02-26 5:15 ` Zhang Yanfei
@ 2013-03-01 1:31 ` Simon Horman
-1 siblings, 0 replies; 16+ messages in thread
From: Simon Horman @ 2013-03-01 1:31 UTC (permalink / raw)
To: Zhang Yanfei; +Cc: Andrew Morton, Eric W. Biederman, kexec, linux-kernel
On Tue, Feb 26, 2013 at 01:15:16PM +0800, Zhang Yanfei wrote:
> The types of the following local variables:
> - ubytes/mbytes in kimage_load_crash_segment()/kimage_load_normal_segment()
> - r in vmcoreinfo_append_str()
> are wrong, so fix them.
>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Simon Horman <horms@verge.net.au>
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
No complaints here.
Reviewed-by: Simon Horman <horms@verge.net.au>
> ---
> kernel/kexec.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 2436ffc..3cbfcc7 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -789,7 +789,7 @@ static int kimage_load_normal_segment(struct kimage *image,
> struct kexec_segment *segment)
> {
> unsigned long maddr;
> - unsigned long ubytes, mbytes;
> + size_t ubytes, mbytes;
> int result;
> unsigned char __user *buf;
>
> @@ -853,7 +853,7 @@ static int kimage_load_crash_segment(struct kimage *image,
> * We do things a page at a time for the sake of kmap.
> */
> unsigned long maddr;
> - unsigned long ubytes, mbytes;
> + size_t ubytes, mbytes;
> int result;
> unsigned char __user *buf;
>
> @@ -1455,7 +1455,7 @@ void vmcoreinfo_append_str(const char *fmt, ...)
> {
> va_list args;
> char buf[0x50];
> - int r;
> + size_t r;
>
> va_start(args, fmt);
> r = vsnprintf(buf, sizeof(buf), fmt, args);
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/2] kexec: fix wrong types of some local variables
@ 2013-03-01 1:31 ` Simon Horman
0 siblings, 0 replies; 16+ messages in thread
From: Simon Horman @ 2013-03-01 1:31 UTC (permalink / raw)
To: Zhang Yanfei; +Cc: Andrew Morton, kexec, Eric W. Biederman, linux-kernel
On Tue, Feb 26, 2013 at 01:15:16PM +0800, Zhang Yanfei wrote:
> The types of the following local variables:
> - ubytes/mbytes in kimage_load_crash_segment()/kimage_load_normal_segment()
> - r in vmcoreinfo_append_str()
> are wrong, so fix them.
>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Simon Horman <horms@verge.net.au>
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
No complaints here.
Reviewed-by: Simon Horman <horms@verge.net.au>
> ---
> kernel/kexec.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 2436ffc..3cbfcc7 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -789,7 +789,7 @@ static int kimage_load_normal_segment(struct kimage *image,
> struct kexec_segment *segment)
> {
> unsigned long maddr;
> - unsigned long ubytes, mbytes;
> + size_t ubytes, mbytes;
> int result;
> unsigned char __user *buf;
>
> @@ -853,7 +853,7 @@ static int kimage_load_crash_segment(struct kimage *image,
> * We do things a page at a time for the sake of kmap.
> */
> unsigned long maddr;
> - unsigned long ubytes, mbytes;
> + size_t ubytes, mbytes;
> int result;
> unsigned char __user *buf;
>
> @@ -1455,7 +1455,7 @@ void vmcoreinfo_append_str(const char *fmt, ...)
> {
> va_list args;
> char buf[0x50];
> - int r;
> + size_t r;
>
> va_start(args, fmt);
> r = vsnprintf(buf, sizeof(buf), fmt, args);
> --
> 1.7.1
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-03-01 1:32 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-26 5:15 [PATCH v3 1/2] kexec: fix wrong types of some local variables Zhang Yanfei
2013-02-26 5:15 ` Zhang Yanfei
2013-02-26 5:30 ` [PATCH v3 2/2] kexec: Use min_t to simplify logic Zhang Yanfei
2013-02-26 5:30 ` Zhang Yanfei
2013-02-26 8:38 ` Joe Perches
2013-02-26 8:38 ` Joe Perches
2013-02-26 8:49 ` Zhang Yanfei
2013-02-26 8:49 ` Zhang Yanfei
2013-02-26 8:53 ` Andrew Morton
2013-02-26 8:53 ` Andrew Morton
2013-02-26 8:55 ` Zhang Yanfei
2013-02-26 8:55 ` Zhang Yanfei
2013-02-27 22:31 ` Andrew Morton
2013-02-27 22:31 ` Andrew Morton
2013-03-01 1:31 ` [PATCH v3 1/2] kexec: fix wrong types of some local variables Simon Horman
2013-03-01 1:31 ` Simon Horman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.