2014-06-13 03:04:46

by Nicholas Krause

Subject: [PATCH] platform/x86/toshiba-apci.c possible bad if test?

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 76441dc..dfd2243 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1238,7 +1238,7 @@ static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
int mode = -1;
int time = -1;

- if (sscanf(buf, "%i", &mode) != 1 && (mode != 2 || mode != 1))
+ if (sscanf(buf, "%i", &mode) != 1 || (mode != 2 || mode != 1))
return -EINVAL;

/* Set the Keyboard Backlight Mode where:
--
1.9.1

2014-06-13 15:21:26

by Azael Avalos

Subject: Re: [PATCH] platform/x86/toshiba-apci.c possible bad if test?

Hi,

I've sent this patch a few weeks ago, but somehow it didn't managed to
get through :-(

If it's still possible, please pick it up Matthew.


Cheers.
Azael


8<------------------------------------------------------------------------------------------------------------------------->8
Intel test builder caught some warnings, one at the
KBD backlight mode store while validating for
correct parameters, and another one that might lead
to not creating the sysfs group

Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c
b/drivers/platform/x86/toshiba_acpi.c
index fbbe46d..f397594 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1218,7 +1218,7 @@ static ssize_t toshiba_kbd_bl_mode_store(struct
device *dev,
int mode = -1;
int time = -1;

- if (sscanf(buf, "%i", &mode) != 1 && (mode != 2 || mode != 1))
+ if (sscanf(buf, "%i", &mode) != 1 || mode > 2 || mode < 1)
return -EINVAL;

/* Set the Keyboard Backlight Mode where:
@@ -1741,7 +1741,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)

ret = sysfs_create_group(&dev->acpi_dev->dev.kobj,
&toshiba_attr_group);
- if (ret) {
+ if (ret != 0) {
dev->sysfs_created = 0;
goto error;
}


--
1.9.1

2014-06-12 21:04 GMT-06:00 Nick <[email protected]>:
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 76441dc..dfd2243 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -1238,7 +1238,7 @@ static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
> int mode = -1;
> int time = -1;
>
> - if (sscanf(buf, "%i", &mode) != 1 && (mode != 2 || mode != 1))
> + if (sscanf(buf, "%i", &mode) != 1 || (mode != 2 || mode != 1))
> return -EINVAL;
>
> /* Set the Keyboard Backlight Mode where:
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
-- El mundo apesta y vosotros apestais tambien --

2014-06-13 21:43:08

by David Rientjes

Subject: Re: [PATCH] platform/x86/toshiba-apci.c possible bad if test?

On Fri, 13 Jun 2014, Azael Avalos wrote:

> Intel test builder caught some warnings, one at the
> KBD backlight mode store while validating for
> correct parameters, and another one that might lead
> to not creating the sysfs group
>
> Signed-off-by: Azael Avalos <[email protected]>
> ---
> drivers/platform/x86/toshiba_acpi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c
> b/drivers/platform/x86/toshiba_acpi.c
> index fbbe46d..f397594 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -1218,7 +1218,7 @@ static ssize_t toshiba_kbd_bl_mode_store(struct
> device *dev,
> int mode = -1;
> int time = -1;
>
> - if (sscanf(buf, "%i", &mode) != 1 && (mode != 2 || mode != 1))
> + if (sscanf(buf, "%i", &mode) != 1 || mode > 2 || mode < 1)
> return -EINVAL;
>
> /* Set the Keyboard Backlight Mode where:
> @@ -1741,7 +1741,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>
> ret = sysfs_create_group(&dev->acpi_dev->dev.kobj,
> &toshiba_attr_group);
> - if (ret) {
> + if (ret != 0) {
> dev->sysfs_created = 0;
> goto error;
> }

It may not have been picked up because you've combined unrelated (and
unnecessary) changes such as your change to toshiba_acpi_add().