Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1380393 > unrolled thread

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

Started byGiedrius Statkevičius <giedrius.statkevicius@gmail.com>
First post2016-04-16 02:10 +0200
Last post2016-04-22 01:10 +0200
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 1/2] asus-laptop: remove redundant initializers Giedrius Statkevičius   <giedrius.statkevicius@gmail.com> - 2016-04-16 02:10 +0200
    [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set Giedrius Statkevičius   <giedrius.statkevicius@gmail.com> - 2016-04-16 02:10 +0200
      Re: [PATCH v2 2/2] asus-laptop: correct error handling in  sysfs_acpi_set Darren Hart <dvhart@infradead.org> - 2016-04-20 22:30 +0200
        Re: [PATCH v2 2/2] asus-laptop: correct error handling in  sysfs_acpi_set Giedrius Statkevičius   <giedrius.statkevicius@gmail.com> - 2016-04-21 08:40 +0200
      Re: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-04-21 22:40 +0200
        Re: [PATCH v2 2/2] asus-laptop: correct error handling in  sysfs_acpi_set Darren Hart <dvhart@infradead.org> - 2016-04-21 23:00 +0200
      Re: [PATCH v2 2/2] asus-laptop: correct error handling in  sysfs_acpi_set Darren Hart <dvhart@infradead.org> - 2016-04-25 19:50 +0200
    Re: [PATCH v2 1/2] asus-laptop: remove redundant initializers Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-04-22 01:10 +0200

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

FromGiedrius Statkevičius <giedrius.statkevicius@gmail.com>
Date2016-04-16 02:10 +0200
Subject[PATCH v2 1/2] asus-laptop: remove redundant initializers
Message-ID<rolYe-347-19@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

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

FromGiedrius Statkevičius <giedrius.statkevicius@gmail.com>
Date2016-04-16 02:10 +0200
Subject[PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set
Message-ID<rolYe-347-25@gated-at.bofh.it>
In reply to#1380393
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

[toc] | [prev] | [next] | [standalone]

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

FromDarren Hart <dvhart@infradead.org>
Date2016-04-20 22:30 +0200
SubjectRe: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set
Message-ID<rq6V5-5pd-11@gated-at.bofh.it>
In reply to#1380396
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

[toc] | [prev] | [next] | [standalone]

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

FromGiedrius Statkevičius <giedrius.statkevicius@gmail.com>
Date2016-04-21 08:40 +0200
SubjectRe: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set
Message-ID<rqgro-4xr-11@gated-at.bofh.it>
In reply to#1383725
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.

[toc] | [prev] | [next] | [standalone]

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

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-04-21 22:40 +0200
SubjectRe: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set
Message-ID<rqtyi-6T4-13@gated-at.bofh.it>
In reply to#1380396
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

[toc] | [prev] | [next] | [standalone]

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

FromDarren Hart <dvhart@infradead.org>
Date2016-04-21 23:00 +0200
SubjectRe: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set
Message-ID<rqtRE-725-7@gated-at.bofh.it>
In reply to#1384561
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

[toc] | [prev] | [next] | [standalone]

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

FromDarren Hart <dvhart@infradead.org>
Date2016-04-25 19:50 +0200
SubjectRe: [PATCH v2 2/2] asus-laptop: correct error handling in sysfs_acpi_set
Message-ID<rrSNY-1nE-3@gated-at.bofh.it>
In reply to#1380396
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

[toc] | [prev] | [next] | [standalone]

#1384624

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-04-22 01:10 +0200
Message-ID<rqvTs-qs-7@gated-at.bofh.it>
In reply to#1380393
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web