Staging: lustre: lproc_osc: Add check on a variable
PATCH 1/1 Staging: lustre: lproc_osc: Add check on a variable
Variable rc is not tested for negative values and hence a check should be included. Also, a check for variable val should be introduced.
--- drivers/staging/lustre/lustre/osc/lproc_osc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/osc/lproc_osc.c +3 -1
@@ -61,7 +61,9 @@ static ssize_t active_store(struct kobject *kobj, struct attribute *attr,unsigned long val; rc =kstrtoul (buffer,10 , &val); -if (rc <0 ) +if (rc) +return rc; +if (val >1 )return -ERANGE ;/* opposite senses */
Signature · 1 line
2.1.0
Quoted 13 lines · Shivani Bhardwaj
Variable rc is not tested for negative values and hence a check should be included. Also, a check for variable val should be introduced.
Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/lustre/lustre/osc/lproc_osc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/staging/lustre/lustre/osc/lproc_osc.c +0
-0
drivers/staging/lustre/lustre/osc/lproc_osc.c static ssize_t active_store(struct kobject *kobj, struct attribute *attr, · line 64
Quoted 5 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +1
-1@@ -61,7 +61,9 @@ static ssize_t active_store(struct kobject *kobj, struct attribute *attr,unsigned long val; rc =kstrtoul (buffer,10 , &val); -if (rc <0 ) +if (rc)
If rc is never > 0, then why do you want to change rc < 0 to just rc?
julia
Quoted 15 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +2
-1+return rc; +if (val >1 )return -ERANGE ;/* opposite senses */ --2.1.0
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151018132030.GA30680%40ubuntu.
For more options, visit https://groups.google.com/d/optout.
Quoted 17 lines · Julia Lawall
Quoted 15 lines · Shivani Bhardwaj
Variable rc is not tested for negative values and hence a check should be included. Also, a check for variable val should be introduced.
Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/lustre/lustre/osc/lproc_osc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/staging/lustre/lustre/osc/lproc_osc.c +0
-0@@ -61,7 +61,9 @@ static ssize_t active_store(struct kobject *kobj, struct attribute *attr,unsigned long val;
Quoted 8 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c static ssize_t active_store(struct kobject *kobj, struct attribute *attr, · line 64
Quoted 3 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +1
-1rc =kstrtoul (buffer,10 , &val); -if (rc <0 ) +if (rc)If rc is never > 0, then why do you want to change rc < 0 to just rc?
julia
Because in case rc=0, it will be success and we don't need to return. This patch is a fix up on earlier patch as suggested by Greg.
Quoted 15 lines
Quoted 15 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +2
-1+return rc; +if (val >1 )return -ERANGE ;/* opposite senses */ --2.1.0
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151018132030.GA30680%40ubuntu.
For more options, visit https://groups.google.com/d/optout.
Quoted 24 lines · Shivani Bhardwaj
Quoted 23 lines · Julia Lawall
Quoted 13 lines · Shivani Bhardwaj
Variable rc is not tested for negative values and hence a check should be included. Also, a check for variable val should be introduced.
Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/lustre/lustre/osc/lproc_osc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/staging/lustre/lustre/osc/lproc_osc.c +0
-0drivers/staging/lustre/lustre/osc/lproc_osc.c static ssize_t active_store(struct kobject *kobj, struct attribute *attr, · line 64
Quoted 5 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +1
-1@@ -61,7 +61,9 @@ static ssize_t active_store(struct kobject *kobj, struct attribute *attr,unsigned long val; rc =kstrtoul (buffer,10 , &val); -if (rc <0 ) +if (rc)If rc is never > 0, then why do you want to change rc < 0 to just rc?
Quoted 5 lines
Quoted 2 lines
julia
Because in case rc=0, it will be success and we don't need to return. This patch is a fix up on earlier patch as suggested by Greg.
I don't follow. When there was rc < 0, there would also not be a return. There are three cases: rc < 0, rc = 0, rc > 0. If you put rc < 0 you will get a return on rc < 0 only. And if you put rc you will get a return on rc < 0 and rc > 0, but rc is never > 0 (you have said - I don't check), so with rc you will also only get a return when rc < 0. Putting rc is a bit more concise. But putting rc < 0 makes it more clear that what is going on is an error check. And I guess it is the code that was there previously, so it is what the developer preferred,
julia
Quoted 24 lines
Quoted 9 lines
Quoted 9 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +2
-1+return rc; +if (val >1 )return -ERANGE ;/* opposite senses */ --2.1.0
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151018132030.GA30680%40ubuntu.
For more options, visit https://groups.google.com/d/optout.--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQEJLf1LXOZr7KS%3DBdC8riOXdjg6jgfDMfV8OK2uXvpJww%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Quoted 43 lines · Julia Lawall
Quoted 24 lines · Shivani Bhardwaj
Quoted 23 lines · Julia Lawall
Quoted 13 lines · Shivani Bhardwaj
Variable rc is not tested for negative values and hence a check should be included. Also, a check for variable val should be introduced.
Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/lustre/lustre/osc/lproc_osc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/staging/lustre/lustre/osc/lproc_osc.c +0
-0drivers/staging/lustre/lustre/osc/lproc_osc.c static ssize_t active_store(struct kobject *kobj, struct attribute *attr, · line 64
Quoted 5 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +1
-1@@ -61,7 +61,9 @@ static ssize_t active_store(struct kobject *kobj, struct attribute *attr,unsigned long val; rc =kstrtoul (buffer,10 , &val); -if (rc <0 ) +if (rc)If rc is never > 0, then why do you want to change rc < 0 to just rc?
Quoted 5 lines
Quoted 2 lines
julia
Because in case rc=0, it will be success and we don't need to return. This patch is a fix up on earlier patch as suggested by Greg.
I don't follow. When there was rc < 0, there would also not be a return. There are three cases: rc < 0, rc = 0, rc > 0. If you put rc < 0 you will get a return on rc < 0 only. And if you put rc you will get a return on rc < 0 and rc > 0, but rc is never > 0 (you have said - I don't check), so with rc you will also only get a return when rc < 0. Putting rc is a bit more concise. But putting rc < 0 makes it more clear that what is going on is an error check. And I guess it is the code that was there previously, so it is what the developer preferred,
Quoted 2 lines
julia
What you say is perfectly fine. But, earlier code mentioned 'if (rc)' only [developer version]. I changed that to 'if(rc<0)' along with some other changes that I got wrong. I've been asked to provide a fix up. If you say, I can change it to rc<0. Yes, it would be more precise. I've consulted Arnd about the positive values. It is never going to happen. Please suggest?
Quoted 24 lines
Quoted 24 lines
Quoted 9 lines
Quoted 9 lines
drivers/staging/lustre/lustre/osc/lproc_osc.c +2
-1+return rc; +if (val >1 )return -ERANGE ;/* opposite senses */ --2.1.0
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151018132030.GA30680%40ubuntu.
For more options, visit https://groups.google.com/d/optout.--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQEJLf1LXOZr7KS%3DBdC8riOXdjg6jgfDMfV8OK2uXvpJww%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Quoted 12 lines
Quoted 11 lines
I don't follow. When there was rc < 0, there would also not be a return. There are three cases: rc < 0, rc = 0, rc > 0. If you put rc < 0 you will get a return on rc < 0 only. And if you put rc you will get a return on rc < 0 and rc > 0, but rc is never > 0 (you have said - I don't check), so with rc you will also only get a return when rc < 0. Putting rc is a bit more concise. But putting rc < 0 makes it more clear that what is going on is an error check. And I guess it is the code that was there previously, so it is what the developer preferred,
julia
Quoted 6 lines
What you say is perfectly fine. But, earlier code mentioned 'if (rc)' only [developer version]. I changed that to 'if(rc<0)' along with some other changes that I got wrong. I've been asked to provide a fix up. If you say, I can change it to rc<0. Yes, it would be more precise. I've consulted Arnd about the positive values. It is never going to happen. Please suggest?
OK, thanks for the explanation. If the code was originally if (rc), it would probably be reasonable to leave it as is. I was thrown off by the fact that there was a - on rc < 0 in the proposed patch. Did Greg apply an incorrect one that you now have to fix up?
julia
Quoted 19 lines · Julia Lawall
Quoted 12 lines
Quoted 11 lines
I don't follow. When there was rc < 0, there would also not be a return. There are three cases: rc < 0, rc = 0, rc > 0. If you put rc < 0 you will get a return on rc < 0 only. And if you put rc you will get a return on rc < 0 and rc > 0, but rc is never > 0 (you have said - I don't check), so with rc you will also only get a return when rc < 0. Putting rc is a bit more concise. But putting rc < 0 makes it more clear that what is going on is an error check. And I guess it is the code that was there previously, so it is what the developer preferred,
julia
Quoted 6 lines
What you say is perfectly fine. But, earlier code mentioned 'if (rc)' only [developer version]. I changed that to 'if(rc<0)' along with some other changes that I got wrong. I've been asked to provide a fix up. If you say, I can change it to rc<0. Yes, it would be more precise. I've consulted Arnd about the positive values. It is never going to happen. Please suggest?
Quoted 6 lines
OK, thanks for the explanation. If the code was originally if (rc), it would probably be reasonable to leave it as is. I was thrown off by the fact that there was a - on rc < 0 in the proposed patch. Did Greg apply an incorrect one that you now have to fix up?
julia
Yes, that's what exactly happened.