The function register_chrdev_region() for count=0 returns a false success.This patch pre
Subsystems (2)
Review
Quoted 1 line · anubhav rakshit
(quoted diff) +1
-0+ if(unlikely(count = 0))
A space between 'if' and '(', please. scripts/checkpatch.pl can warn you about this kind of thing.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH] The function register_chrdev_region() for count=0
Quoted 21 lines · anubhav rakshit
Signed-off-by Anubhav Rakshit <[email protected]>--- fs/char_dev.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)fs/char_dev.c +4
-1@@ -199,7 +199,10 @@ int register_chrdev_region(dev_t from, unsigned count, const char *name)struct char_device_struct *cd;dev_t to = from + count;dev_t n, next; - + +if (unlikely (count =0 )) +return -EINVAL ; +for (n = from; n < to; n = next) { next =MKDEV (MAJOR (n)+1 ,0 );
Quoted 1 line
if (next > to)
I'd say
a) unlikely in such rarely used function is useless
b)
register_chrdev_region(from, 0, name); unregister_chrdev_region(from, 0);
will unregister region sucessfully ;-)
c) this is problem of a caller -- don't!
PATCH 1/1 The function register_chrdev_region() for count=0 returns a false success.This patch prevent
In reply to Matthew Wilcox--- fs/char_dev.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
fs/char_dev.c +0 -0
@@ -199,7 +199,10 @@ int register_chrdev_region(dev_t from, unsigned
count, const char *name)
struct char_device_struct *cd;
dev_t to = from + count;
dev_t n, next;
-
+
+ if(unlikely(count = 0))
+ return -EINVAL;
+
for (n = from; n < to; n = next) {
next = MKDEV(MAJOR(n)+1, 0);
if (next > to)
Signature · 2 lines
-- Anubhav Rakshit
Quoted 29 lines · Alexey Dobriyan
Quoted 21 lines · anubhav rakshit
Signed-off-by Anubhav Rakshit <[email protected]>--- fs/char_dev.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)fs/char_dev.c +4
-1@@ -199,7 +199,10 @@ int register_chrdev_region(dev_t from, unsigned count, const char *name)struct char_device_struct *cd;dev_t to = from + count;dev_t n, next; - + +if (unlikely (count =0 )) +return -EINVAL ; +for (n = from; n < to; n = next) { next =MKDEV (MAJOR (n)+1 ,0 );Quoted 1 line
if (next > to)
I'd say
a) unlikely in such rarely used function is useless
b)
Quoted 4 lines
register_chrdev_region(from, 0, name); unregister_chrdev_region(from, 0);will unregister region sucessfully ;-)
register_chrdev_region() doesnot actually succeed with count=0.The
caller should be notified about this.
Quoted 2 lines
c) this is problem of a caller -- don't!
Exactly and he/she should be notified.
Quoted 2 lines
Signature · 1 line
Anubhav Rakshit
<--snip-->
Quoted 2 lines
A space between 'if' and '(', please.
scripts/checkpatch.plcan warn you about this kind of thing.
<--snip-->
Resending with corrections
--- fs/char_dev.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
fs/char_dev.c +0 -0
@@ -200,6 +200,9 @@ int register_chrdev_region(dev_t from, unsigned
count, const char *name)
dev_t to = from + count;
dev_t n, next;
+ if (unlikely(count = 0))
+ return -EINVAL;
+
for (n = from; n < to; n = next) {
next = MKDEV(MAJOR(n)+1, 0);
if (next > to)
--