Initializing rv to AE_OK is pointless because later function results are
assigned to them and only then the variable is used
Signed-off-by: Giedrius Statkevičius <[email protected]>
---
drivers/platform/x86/asus-laptop.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index f2b5d0a..d86d42e 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -771,7 +771,7 @@ static int asus_read_brightness(struct backlight_device *bd)
{
struct asus_laptop *asus = bl_get_data(bd);
unsigned long long value;
- acpi_status rv = AE_OK;
+ acpi_status rv;
rv = acpi_evaluate_integer(asus->handle, METHOD_BRIGHTNESS_GET,
NULL, &value);
@@ -865,7 +865,7 @@ static ssize_t infos_show(struct device *dev, struct device_attribute *attr,
int len = 0;
unsigned long long temp;
char buf[16]; /* enough for all info */
- acpi_status rv = AE_OK;
+ acpi_status rv;
/*
* We use the easy way, we don't care of off and count,
@@ -1265,7 +1265,7 @@ static DEVICE_ATTR_RO(ls_value);
static int asus_gps_status(struct asus_laptop *asus)
{
unsigned long long status;
- acpi_status rv = AE_OK;
+ acpi_status rv;
rv = acpi_evaluate_integer(asus->handle, METHOD_GPS_STATUS,
NULL, &status);
--
2.8.0
Properly return rv back to the caller in the case of an error in
parse_arg. In the process remove a unused variable 'out'.
Signed-off-by: Giedrius Statkevičius <[email protected]>
---
drivers/platform/x86/asus-laptop.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index d86d42e..9a69734 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -946,11 +946,10 @@ static ssize_t sysfs_acpi_set(struct asus_laptop *asus,
const char *method)
{
int rv, value;
- int out = 0;
rv = parse_arg(buf, count, &value);
- if (rv > 0)
- out = value ? 1 : 0;
+ if (rv <= 0)
+ return rv;
if (write_acpi_int(asus->handle, method, value))
return -ENODEV;
--
2.8.0
On Sat, Apr 16, 2016 at 03:01:57AM +0300, Giedrius Statkevičius wrote:
> Properly return rv back to the caller in the case of an error in
> parse_arg. In the process remove a unused variable 'out'.
The initial problem if I recall was value being uninitialized. Is that correct?
>
> Signed-off-by: Giedrius Statkevičius <[email protected]>
> ---
> drivers/platform/x86/asus-laptop.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index d86d42e..9a69734 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -946,11 +946,10 @@ static ssize_t sysfs_acpi_set(struct asus_laptop *asus,
> const char *method)
> {
> int rv, value;
> - int out = 0;
>
> rv = parse_arg(buf, count, &value);
> - if (rv > 0)
> - out = value ? 1 : 0;
> + if (rv <= 0)
> + return rv;
>
> if (write_acpi_int(asus->handle, method, value))
> return -ENODEV;
> --
> 2.8.0
>
>
--
Darren Hart
Intel Open Source Technology Center
On Wed, Apr 20, 2016 at 01:19:55PM -0700, Darren Hart wrote:
> On Sat, Apr 16, 2016 at 03:01:57AM +0300, Giedrius Statkevičius wrote:
> > Properly return rv back to the caller in the case of an error in
> > parse_arg. In the process remove a unused variable 'out'.
>
> The initial problem if I recall was value being uninitialized. Is that correct?
No, 'out' was just removed as it was unused. Then you caught the issue with
error handling in this function so I've updated this patch to fix that as well.
On Sat, Apr 16, 2016 at 3:01 AM, Giedrius Statkevičius
<[email protected]> wrote:
> Properly return rv back to the caller in the case of an error in
> parse_arg. In the process remove a unused variable 'out'.
>
> Signed-off-by: Giedrius Statkevičius <[email protected]>
> ---
> drivers/platform/x86/asus-laptop.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index d86d42e..9a69734 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -946,11 +946,10 @@ static ssize_t sysfs_acpi_set(struct asus_laptop *asus,
> const char *method)
> {
> int rv, value;
> - int out = 0;
>
> rv = parse_arg(buf, count, &value);
Just noticed (might be a separate patch for this) that parse_arg
pretty much duplicated kstrotint().
> - if (rv > 0)
> - out = value ? 1 : 0;
> + if (rv <= 0)
> + return rv;
>
> if (write_acpi_int(asus->handle, method, value))
> return -ENODEV;
> --
> 2.8.0
>
--
With Best Regards,
Andy Shevchenko
On Thu, Apr 21, 2016 at 11:34:13PM +0300, Andy Shevchenko wrote:
> On Sat, Apr 16, 2016 at 3:01 AM, Giedrius Statkevičius
> <[email protected]> wrote:
> > Properly return rv back to the caller in the case of an error in
> > parse_arg. In the process remove a unused variable 'out'.
> >
> > Signed-off-by: Giedrius Statkevičius <[email protected]>
> > ---
> > drivers/platform/x86/asus-laptop.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> > index d86d42e..9a69734 100644
> > --- a/drivers/platform/x86/asus-laptop.c
> > +++ b/drivers/platform/x86/asus-laptop.c
> > @@ -946,11 +946,10 @@ static ssize_t sysfs_acpi_set(struct asus_laptop *asus,
> > const char *method)
> > {
> > int rv, value;
> > - int out = 0;
> >
> > rv = parse_arg(buf, count, &value);
>
> Just noticed (might be a separate patch for this) that parse_arg
> pretty much duplicated kstrotint().
Ah, thanks Andy.
I'd like to take this one as is, but a cleanup for that would be welcome indeed.
--
Darren Hart
Intel Open Source Technology Center
On Sat, Apr 16, 2016 at 3:01 AM, Giedrius Statkevičius
<[email protected]> wrote:
> Initializing rv to AE_OK is pointless because later function results are
> assigned to them and only then the variable is used
>
> Signed-off-by: Giedrius Statkevičius <[email protected]>
Fine to me:
Acked-by: Andy Shevchenko <[email protected]>
> ---
> drivers/platform/x86/asus-laptop.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index f2b5d0a..d86d42e 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -771,7 +771,7 @@ static int asus_read_brightness(struct backlight_device *bd)
> {
> struct asus_laptop *asus = bl_get_data(bd);
> unsigned long long value;
> - acpi_status rv = AE_OK;
> + acpi_status rv;
>
> rv = acpi_evaluate_integer(asus->handle, METHOD_BRIGHTNESS_GET,
> NULL, &value);
> @@ -865,7 +865,7 @@ static ssize_t infos_show(struct device *dev, struct device_attribute *attr,
> int len = 0;
> unsigned long long temp;
> char buf[16]; /* enough for all info */
> - acpi_status rv = AE_OK;
> + acpi_status rv;
>
> /*
> * We use the easy way, we don't care of off and count,
> @@ -1265,7 +1265,7 @@ static DEVICE_ATTR_RO(ls_value);
> static int asus_gps_status(struct asus_laptop *asus)
> {
> unsigned long long status;
> - acpi_status rv = AE_OK;
> + acpi_status rv;
>
> rv = acpi_evaluate_integer(asus->handle, METHOD_GPS_STATUS,
> NULL, &status);
> --
> 2.8.0
>
--
With Best Regards,
Andy Shevchenko
On Sat, Apr 16, 2016 at 03:01:57AM +0300, Giedrius Statkevičius wrote:
> Properly return rv back to the caller in the case of an error in
> parse_arg. In the process remove a unused variable 'out'.
>
> Signed-off-by: Giedrius Statkevičius <[email protected]>
1 and 2 queued for 4.7.
> ---
> drivers/platform/x86/asus-laptop.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index d86d42e..9a69734 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -946,11 +946,10 @@ static ssize_t sysfs_acpi_set(struct asus_laptop *asus,
> const char *method)
> {
> int rv, value;
> - int out = 0;
>
> rv = parse_arg(buf, count, &value);
> - if (rv > 0)
> - out = value ? 1 : 0;
> + if (rv <= 0)
> + return rv;
>
> if (write_acpi_int(asus->handle, method, value))
> return -ENODEV;
> --
> 2.8.0
>
>
--
Darren Hart
Intel Open Source Technology Center