* Re: [PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.
@ 2012-11-22 0:23 ` Rafael J. Wysocki
0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-11-22 0:23 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: linux-kernel, linux-acpi, lenb, kernel-janitors
On Thursday, November 22, 2012 01:05:30 AM Cyril Roelandt wrote:
> On 11/21/2012 01:44 AM, Rafael J. Wysocki wrote:
> > On Saturday, November 17, 2012 02:54:23 AM Cyril Roelandt wrote:
> >> The LEN variable is unsigned, therefore checking whether it is less than 0 is
> >> useless.
> >>
> >> Signed-off-by: Cyril Roelandt<tipecaml@gmail.com>
> >> ---
> >> drivers/acpi/proc.c | 2 --
> >> 1 file changed, 2 deletions(-)
> >>
> >> diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
> >> index 27adb09..37871a7 100644
> >> --- a/drivers/acpi/proc.c
> >> +++ b/drivers/acpi/proc.c
> >> @@ -366,8 +366,6 @@ acpi_system_write_wakeup_device(struct file *file,
> >>
> >> if (len> 4)
> >> len = 4;
> >> - if (len< 0)
> >> - return -EFAULT;
> >
> > I would prefer to check if count< 0 instead.
> >
>
>
> count has type size_t, so it is also an unsigned variable. I think that
> acpi_system_write_wakeup_device really cannot be called with a "count"
> parameter that is less than 0.
OK
So I suppose we can just drop len entirely and replace it with count, right?
[And drop the < 0 check too.]
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.
2012-11-22 0:23 ` Rafael J. Wysocki
@ 2012-11-22 0:30 ` Cyril Roelandt
-1 siblings, 0 replies; 14+ messages in thread
From: Cyril Roelandt @ 2012-11-22 0:30 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-kernel, linux-acpi, lenb, kernel-janitors
On 11/22/2012 01:23 AM, Rafael J. Wysocki wrote:
> On Thursday, November 22, 2012 01:05:30 AM Cyril Roelandt wrote:
>> On 11/21/2012 01:44 AM, Rafael J. Wysocki wrote:
>>> On Saturday, November 17, 2012 02:54:23 AM Cyril Roelandt wrote:
>>>> The LEN variable is unsigned, therefore checking whether it is less than 0 is
>>>> useless.
>>>>
>>>> Signed-off-by: Cyril Roelandt<tipecaml@gmail.com>
>>>> ---
>>>> drivers/acpi/proc.c | 2 --
>>>> 1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
>>>> index 27adb09..37871a7 100644
>>>> --- a/drivers/acpi/proc.c
>>>> +++ b/drivers/acpi/proc.c
>>>> @@ -366,8 +366,6 @@ acpi_system_write_wakeup_device(struct file *file,
>>>>
>>>> if (len> 4)
>>>> len = 4;
>>>> - if (len< 0)
>>>> - return -EFAULT;
>>>
>>> I would prefer to check if count< 0 instead.
>>>
>>
>>
>> count has type size_t, so it is also an unsigned variable. I think that
>> acpi_system_write_wakeup_device really cannot be called with a "count"
>> parameter that is less than 0.
>
> OK
>
> So I suppose we can just drop len entirely and replace it with count, right?
> [And drop the< 0 check too.]
>
Indeed, I'll resend.
Cyril Roelandt.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.
@ 2012-11-22 0:30 ` Cyril Roelandt
0 siblings, 0 replies; 14+ messages in thread
From: Cyril Roelandt @ 2012-11-22 0:30 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-kernel, linux-acpi, lenb, kernel-janitors
On 11/22/2012 01:23 AM, Rafael J. Wysocki wrote:
> On Thursday, November 22, 2012 01:05:30 AM Cyril Roelandt wrote:
>> On 11/21/2012 01:44 AM, Rafael J. Wysocki wrote:
>>> On Saturday, November 17, 2012 02:54:23 AM Cyril Roelandt wrote:
>>>> The LEN variable is unsigned, therefore checking whether it is less than 0 is
>>>> useless.
>>>>
>>>> Signed-off-by: Cyril Roelandt<tipecaml@gmail.com>
>>>> ---
>>>> drivers/acpi/proc.c | 2 --
>>>> 1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
>>>> index 27adb09..37871a7 100644
>>>> --- a/drivers/acpi/proc.c
>>>> +++ b/drivers/acpi/proc.c
>>>> @@ -366,8 +366,6 @@ acpi_system_write_wakeup_device(struct file *file,
>>>>
>>>> if (len> 4)
>>>> len = 4;
>>>> - if (len< 0)
>>>> - return -EFAULT;
>>>
>>> I would prefer to check if count< 0 instead.
>>>
>>
>>
>> count has type size_t, so it is also an unsigned variable. I think that
>> acpi_system_write_wakeup_device really cannot be called with a "count"
>> parameter that is less than 0.
>
> OK
>
> So I suppose we can just drop len entirely and replace it with count, right?
> [And drop the< 0 check too.]
>
Indeed, I'll resend.
Cyril Roelandt.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] acpi_system_write_wakeup_device(): fix error check for unsigned variable.
2012-11-22 0:23 ` Rafael J. Wysocki
@ 2012-11-22 0:34 ` Cyril Roelandt
-1 siblings, 0 replies; 14+ messages in thread
From: Cyril Roelandt @ 2012-11-22 0:34 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-acpi, rjw, lenb, kernel-janitors, Cyril Roelandt
The LEN variable is unsigned, therefore checking whether it is less than 0 is
useless. Also drop the LEN variable, since the COUNT parameter can be used
instead.
Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
---
drivers/acpi/proc.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
index 27adb09..ef98796 100644
--- a/drivers/acpi/proc.c
+++ b/drivers/acpi/proc.c
@@ -362,16 +362,13 @@ acpi_system_write_wakeup_device(struct file *file,
struct list_head *node, *next;
char strbuf[5];
char str[5] = "";
- unsigned int len = count;
- if (len > 4)
- len = 4;
- if (len < 0)
- return -EFAULT;
+ if (count > 4)
+ count = 4;
- if (copy_from_user(strbuf, buffer, len))
+ if (copy_from_user(strbuf, buffer, count))
return -EFAULT;
- strbuf[len] = '\0';
+ strbuf[count] = '\0';
sscanf(strbuf, "%s", str);
mutex_lock(&acpi_device_lock);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2] acpi_system_write_wakeup_device(): fix error check for unsigned variable.
@ 2012-11-22 0:34 ` Cyril Roelandt
0 siblings, 0 replies; 14+ messages in thread
From: Cyril Roelandt @ 2012-11-22 0:34 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-acpi, rjw, lenb, kernel-janitors, Cyril Roelandt
The LEN variable is unsigned, therefore checking whether it is less than 0 is
useless. Also drop the LEN variable, since the COUNT parameter can be used
instead.
Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
---
drivers/acpi/proc.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
index 27adb09..ef98796 100644
--- a/drivers/acpi/proc.c
+++ b/drivers/acpi/proc.c
@@ -362,16 +362,13 @@ acpi_system_write_wakeup_device(struct file *file,
struct list_head *node, *next;
char strbuf[5];
char str[5] = "";
- unsigned int len = count;
- if (len > 4)
- len = 4;
- if (len < 0)
- return -EFAULT;
+ if (count > 4)
+ count = 4;
- if (copy_from_user(strbuf, buffer, len))
+ if (copy_from_user(strbuf, buffer, count))
return -EFAULT;
- strbuf[len] = '\0';
+ strbuf[count] = '\0';
sscanf(strbuf, "%s", str);
mutex_lock(&acpi_device_lock);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] acpi_system_write_wakeup_device(): fix error check for unsigned variable.
2012-11-22 0:34 ` Cyril Roelandt
@ 2012-11-22 22:40 ` Rafael J. Wysocki
-1 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-11-22 22:40 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: linux-kernel, linux-acpi, lenb, kernel-janitors
On Thursday, November 22, 2012 01:34:55 AM Cyril Roelandt wrote:
> The LEN variable is unsigned, therefore checking whether it is less than 0 is
> useless. Also drop the LEN variable, since the COUNT parameter can be used
> instead.
Applied (under a different subject).
Thanks,
Rafael
> Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
> ---
> drivers/acpi/proc.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
> index 27adb09..ef98796 100644
> --- a/drivers/acpi/proc.c
> +++ b/drivers/acpi/proc.c
> @@ -362,16 +362,13 @@ acpi_system_write_wakeup_device(struct file *file,
> struct list_head *node, *next;
> char strbuf[5];
> char str[5] = "";
> - unsigned int len = count;
>
> - if (len > 4)
> - len = 4;
> - if (len < 0)
> - return -EFAULT;
> + if (count > 4)
> + count = 4;
>
> - if (copy_from_user(strbuf, buffer, len))
> + if (copy_from_user(strbuf, buffer, count))
> return -EFAULT;
> - strbuf[len] = '\0';
> + strbuf[count] = '\0';
> sscanf(strbuf, "%s", str);
>
> mutex_lock(&acpi_device_lock);
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] acpi_system_write_wakeup_device(): fix error check for unsigned variable.
@ 2012-11-22 22:40 ` Rafael J. Wysocki
0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-11-22 22:40 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: linux-kernel, linux-acpi, lenb, kernel-janitors
On Thursday, November 22, 2012 01:34:55 AM Cyril Roelandt wrote:
> The LEN variable is unsigned, therefore checking whether it is less than 0 is
> useless. Also drop the LEN variable, since the COUNT parameter can be used
> instead.
Applied (under a different subject).
Thanks,
Rafael
> Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
> ---
> drivers/acpi/proc.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c
> index 27adb09..ef98796 100644
> --- a/drivers/acpi/proc.c
> +++ b/drivers/acpi/proc.c
> @@ -362,16 +362,13 @@ acpi_system_write_wakeup_device(struct file *file,
> struct list_head *node, *next;
> char strbuf[5];
> char str[5] = "";
> - unsigned int len = count;
>
> - if (len > 4)
> - len = 4;
> - if (len < 0)
> - return -EFAULT;
> + if (count > 4)
> + count = 4;
>
> - if (copy_from_user(strbuf, buffer, len))
> + if (copy_from_user(strbuf, buffer, count))
> return -EFAULT;
> - strbuf[len] = '\0';
> + strbuf[count] = '\0';
> sscanf(strbuf, "%s", str);
>
> mutex_lock(&acpi_device_lock);
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 14+ messages in thread