[PATCH v2 1/2] asus-laptop: remove redundant initializers

[PATCH v2 1/2] asus-laptop: remove redundant initializers

[Giedrius Statkevičius]

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 <giedrius.statkevicius@gmail.com>
---
 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

[PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set

[Giedrius Statkevičius]

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 <giedrius.statkevicius@gmail.com>
---
 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

Re: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set

[Darren Hart]

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 <giedrius.statkevicius@gmail.com>
> ---
>  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

Re: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set

[Giedrius Statkevičius]

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.

Re: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set

[Andy Shevchenko]

On Sat, Apr 16, 2016 at 3:01 AM, Giedrius Statkevičius
<giedrius.statkevicius@gmail.com> 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 <giedrius.statkevicius@gmail.com>
> ---
>  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

Re: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set

[Darren Hart]

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
> <giedrius.statkevicius@gmail.com> 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 <giedrius.statkevicius@gmail.com>
> > ---
> >  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

Re: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set

[Darren Hart]

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 <giedrius.statkevicius@gmail.com>

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

Re: [PATCH v2 1/2] asus-laptop: remove redundant initializers

[Andy Shevchenko]

On Sat, Apr 16, 2016 at 3:01 AM, Giedrius Statkevičius
<giedrius.statkevicius@gmail.com> 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 <giedrius.statkevicius@gmail.com>

Fine to me:
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> ---
>  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