[PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[Cyril Roelandt]

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;
 
 	if (copy_from_user(strbuf, buffer, len))
 		return -EFAULT;
-- 
1.7.10.4

Re: [PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[Rafael J. Wysocki]

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.

>  
>  	if (copy_from_user(strbuf, buffer, len))
>  		return -EFAULT;

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[Cyril Roelandt]

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.

WBR,
Cyril Roelandt.

Re: [PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[Rafael J. Wysocki]

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.

Re: [PATCH] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[Cyril Roelandt]

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.

[PATCH v2] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[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

Re: [PATCH v2] acpi_system_write_wakeup_device(): fix error check for unsigned variable.

[Rafael J. Wysocki]

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.