No functional changes requires that we eat errors from strtobool.
If people want to not do this, then it should be fixed at a later date.
V2: Simplification suggested by Rusty Russell removes the need for
additional variable ret.
Signed-off-by: Jonathan Cameron <[email protected]>
---
fs/debugfs/file.c | 17 ++++-------------
1 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 89d394d..568304d 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -429,25 +429,16 @@ static ssize_t write_file_bool(struct file *file, const char __user *user_buf,
{
char buf[32];
int buf_size;
+ bool bv;
u32 *val = file->private_data;
buf_size = min(count, (sizeof(buf)-1));
if (copy_from_user(buf, user_buf, buf_size))
return -EFAULT;
- switch (buf[0]) {
- case 'y':
- case 'Y':
- case '1':
- *val = 1;
- break;
- case 'n':
- case 'N':
- case '0':
- *val = 0;
- break;
- }
-
+ if (strtobool(buf, &bv) == 0)
+ *val = bv;
+
return count;
}
--
1.7.3.4
On 04/19/2011 11:43 PM, Jonathan Cameron wrote:
> No functional changes requires that we eat errors from strtobool.
> If people want to not do this, then it should be fixed at a later date.
May as well fix it now or it will get forgotten about. A second patch on
top of this can fix the bug.
~Ryan
> V2: Simplification suggested by Rusty Russell removes the need for
> additional variable ret.
>
> Signed-off-by: Jonathan Cameron <[email protected]>
> ---
> fs/debugfs/file.c | 17 ++++-------------
> 1 files changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 89d394d..568304d 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -429,25 +429,16 @@ static ssize_t write_file_bool(struct file *file, const char __user *user_buf,
> {
> char buf[32];
> int buf_size;
> + bool bv;
> u32 *val = file->private_data;
>
> buf_size = min(count, (sizeof(buf)-1));
> if (copy_from_user(buf, user_buf, buf_size))
> return -EFAULT;
>
> - switch (buf[0]) {
> - case 'y':
> - case 'Y':
> - case '1':
> - *val = 1;
> - break;
> - case 'n':
> - case 'N':
> - case '0':
> - *val = 0;
> - break;
> - }
> -
> + if (strtobool(buf, &bv) == 0)
> + *val = bv;
> +
> return count;
> }
>
--
Bluewater Systems Ltd - ARM Technology Solution Centre
Ryan Mallon 5 Amuri Park, 404 Barbadoes St
[email protected] PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934
On 04/19/11 21:30, Ryan Mallon wrote:
> On 04/19/2011 11:43 PM, Jonathan Cameron wrote:
>> No functional changes requires that we eat errors from strtobool.
>> If people want to not do this, then it should be fixed at a later date.
>
> May as well fix it now or it will get forgotten about. A second patch on
> top of this can fix the bug.
What worries me about this 'fix' is that it may well break some 'interesting'
bit of userspace code. It would count as a userspace api change, be it
a fairly minor one.
Greg, do you thing it's worth returning an error on a non bool value?
>
> ~Ryan
>
>> V2: Simplification suggested by Rusty Russell removes the need for
>> additional variable ret.
>>
>> Signed-off-by: Jonathan Cameron <[email protected]>
>> ---
>> fs/debugfs/file.c | 17 ++++-------------
>> 1 files changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
>> index 89d394d..568304d 100644
>> --- a/fs/debugfs/file.c
>> +++ b/fs/debugfs/file.c
>> @@ -429,25 +429,16 @@ static ssize_t write_file_bool(struct file *file, const char __user *user_buf,
>> {
>> char buf[32];
>> int buf_size;
>> + bool bv;
>> u32 *val = file->private_data;
>>
>> buf_size = min(count, (sizeof(buf)-1));
>> if (copy_from_user(buf, user_buf, buf_size))
>> return -EFAULT;
>>
>> - switch (buf[0]) {
>> - case 'y':
>> - case 'Y':
>> - case '1':
>> - *val = 1;
>> - break;
>> - case 'n':
>> - case 'N':
>> - case '0':
>> - *val = 0;
>> - break;
>> - }
>> -
>> + if (strtobool(buf, &bv) == 0)
>> + *val = bv;
>> +
>> return count;
>> }
>>
>
>
On Wed, Apr 20, 2011 at 10:33:22AM +0100, Jonathan Cameron wrote:
> On 04/19/11 21:30, Ryan Mallon wrote:
> > On 04/19/2011 11:43 PM, Jonathan Cameron wrote:
> >> No functional changes requires that we eat errors from strtobool.
> >> If people want to not do this, then it should be fixed at a later date.
> >
> > May as well fix it now or it will get forgotten about. A second patch on
> > top of this can fix the bug.
> What worries me about this 'fix' is that it may well break some 'interesting'
> bit of userspace code. It would count as a userspace api change, be it
> a fairly minor one.
>
> Greg, do you thing it's worth returning an error on a non bool value?
No one has ever complained about it, so I doubt it's a big issue.
So I'd say to just leave it as-is for now.
thanks,
greg k-h
On 04/26/2011 10:54 AM, Greg KH wrote:
> On Wed, Apr 20, 2011 at 10:33:22AM +0100, Jonathan Cameron wrote:
>> On 04/19/11 21:30, Ryan Mallon wrote:
>>> On 04/19/2011 11:43 PM, Jonathan Cameron wrote:
>>>> No functional changes requires that we eat errors from strtobool.
>>>> If people want to not do this, then it should be fixed at a later date.
>>>
>>> May as well fix it now or it will get forgotten about. A second patch on
>>> top of this can fix the bug.
>> What worries me about this 'fix' is that it may well break some 'interesting'
>> bit of userspace code. It would count as a userspace api change, be it
>> a fairly minor one.
>>
>> Greg, do you thing it's worth returning an error on a non bool value?
>
> No one has ever complained about it, so I doubt it's a big issue.
Probably because nobody checks for errors in user space :-p
~Ryan
--
Bluewater Systems Ltd - ARM Technology Solution Centre
Ryan Mallon 5 Amuri Park, 404 Barbadoes St
[email protected] PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934