linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Sony-laptop: Use common error handling code in sony_nc_setup_rfkill()
@ 2017-10-30 20:15 SF Markus Elfring
  2017-10-31 13:33 ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2017-10-30 20:15 UTC (permalink / raw)
  To: platform-driver-x86, Andy Shevchenko, Darren Hart, Mattia Dongili
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 30 Oct 2017 21:10:49 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/platform/x86/sony-laptop.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index a16cea2be9c3..3ab6e41a8ed6 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -1660,18 +1660,16 @@ static int sony_nc_setup_rfkill(struct acpi_device *device,
 	if (!rfk)
 		return -ENOMEM;
 
-	if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0) {
-		rfkill_destroy(rfk);
-		return -1;
-	}
+	if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0)
+		goto destroy_rfk;
+
 	hwblock = !(result & 0x1);
 
 	if (sony_call_snc_handle(sony_rfkill_handle,
-				sony_rfkill_address[nc_type],
-				&result) < 0) {
-		rfkill_destroy(rfk);
-		return -1;
-	}
+				 sony_rfkill_address[nc_type],
+				 &result) < 0)
+		goto destroy_rfk;
+
 	swblock = !(result & 0x2);
 
 	rfkill_init_sw_state(rfk, swblock);
@@ -1684,6 +1682,10 @@ static int sony_nc_setup_rfkill(struct acpi_device *device,
 	}
 	sony_rfkill_devices[nc_type] = rfk;
 	return err;
+
+destroy_rfk:
+	rfkill_destroy(rfk);
+	return -1;
 }
 
 static void sony_nc_rfkill_update(void)
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] Sony-laptop: Use common error handling code in sony_nc_setup_rfkill()
  2017-10-30 20:15 [PATCH] Sony-laptop: Use common error handling code in sony_nc_setup_rfkill() SF Markus Elfring
@ 2017-10-31 13:33 ` Andy Shevchenko
  2017-10-31 14:24   ` SF Markus Elfring
  2017-11-01 18:45   ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() SF Markus Elfring
  0 siblings, 2 replies; 19+ messages in thread
From: Andy Shevchenko @ 2017-10-31 13:33 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Platform Driver, Andy Shevchenko, Darren Hart, Mattia Dongili,
	LKML, kernel-janitors

On Mon, Oct 30, 2017 at 10:15 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 30 Oct 2017 21:10:49 +0100
>
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
>
> This issue was detected by using the Coccinelle software.

Apparently this patch done without reading the actual code.

NAK.

-1 is EPERM which sounds wrong here. If you would like to fix it,
propagate a real error codes from sony_call_snc_handle().

>         if (!rfk)
>                 return -ENOMEM;

Okay error code.

>
> -       if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0) {
> -               rfkill_destroy(rfk);
> -               return -1;

Not okay.

> -       }
> +       if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0)
> +               goto destroy_rfk;
> +
>         hwblock = !(result & 0x1);
>
>         if (sony_call_snc_handle(sony_rfkill_handle,
> -                               sony_rfkill_address[nc_type],
> -                               &result) < 0) {
> -               rfkill_destroy(rfk);
> -               return -1;

Not okay and it might be different from previous case.

> -       }
> +                                sony_rfkill_address[nc_type],
> +                                &result) < 0)
> +               goto destroy_rfk;
> +
>         swblock = !(result & 0x2);
>
>         rfkill_init_sw_state(rfk, swblock);
> @@ -1684,6 +1682,10 @@ static int sony_nc_setup_rfkill(struct acpi_device *device,
>         }
>         sony_rfkill_devices[nc_type] = rfk;
>         return err;
> +
> +destroy_rfk:
> +       rfkill_destroy(rfk);
> +       return -1;

P.S. Don't bother us with patches on which you didn't do your home work.
Thanks for understanding.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Sony-laptop: Use common error handling code in sony_nc_setup_rfkill()
  2017-10-31 13:33 ` Andy Shevchenko
@ 2017-10-31 14:24   ` SF Markus Elfring
  2017-10-31 16:35     ` Andy Shevchenko
  2017-11-01 18:45   ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() SF Markus Elfring
  1 sibling, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2017-10-31 14:24 UTC (permalink / raw)
  To: Andy Shevchenko, platform-driver-x86
  Cc: Andy Shevchenko, Darren Hart, Mattia Dongili, LKML, kernel-janitors

> Apparently this patch done without reading the actual code.

I performed another simple software refactoring.


> NAK.
> 
> -1 is EPERM which sounds wrong here. If you would like to fix it,
> propagate a real error codes from sony_call_snc_handle().

I do not know at the moment why an error code which you find
questionable was suggested by Marco Chiappero on 2012-05-19
and committed by Matthew Garrett on 2012-05-31 (according the
commit d6f15ed876b83a1a0eba1d0473eef58acc95444a: use soft rfkill
status stored in hw).


>>         if (!rfk)
>>                 return -ENOMEM;
> 
> Okay error code.

Can any other identifier make more sense there?


>> -       }
>> +       if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0)
>> +               goto destroy_rfk;
>> +
>>         hwblock = !(result & 0x1);
>>
>>         if (sony_call_snc_handle(sony_rfkill_handle,
>> -                               sony_rfkill_address[nc_type],
>> -                               &result) < 0) {
>> -               rfkill_destroy(rfk);
>> -               return -1;
> 
> Not okay and it might be different from previous case.

The shown function call was repeated with an other parameter.
Which error reporting would you find more appropriate then?


> P.S. Don't bother us with patches on which you didn't do your home work.

Do we occasionally need to distinguish better between an ordinary
refactoring and the adjustments for additional software improvements?

Regards,
Markus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] Sony-laptop: Use common error handling code in sony_nc_setup_rfkill()
  2017-10-31 14:24   ` SF Markus Elfring
@ 2017-10-31 16:35     ` Andy Shevchenko
  2017-10-31 16:56       ` SF Markus Elfring
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2017-10-31 16:35 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Platform Driver, Andy Shevchenko, Darren Hart, Mattia Dongili,
	LKML, kernel-janitors

On Tue, Oct 31, 2017 at 4:24 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> Apparently this patch done without reading the actual code.
>
> I performed another simple software refactoring.

Which is _useless_ per se by the reasons you had been told already
_several_ times about.

Either you do a real fix (propagate correct error codes to upper
layers), or just don't bother at all.

>> NAK.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Sony-laptop: Use common error handling code in sony_nc_setup_rfkill()
  2017-10-31 16:35     ` Andy Shevchenko
@ 2017-10-31 16:56       ` SF Markus Elfring
  0 siblings, 0 replies; 19+ messages in thread
From: SF Markus Elfring @ 2017-10-31 16:56 UTC (permalink / raw)
  To: Andy Shevchenko, platform-driver-x86
  Cc: Andy Shevchenko, Darren Hart, Mattia Dongili, LKML, kernel-janitors

>> I performed another simple software refactoring.
> 
> Which is _useless_ per se

I disagree in this case because the goal of a refactoring should be
to adjust something in a desired direction without changing
the provided functionality.


> by the reasons you had been told already _several_ times about.

There can be other reasons why you do not like a concrete change.


> Either you do a real fix (propagate correct error codes
> to upper layers), or just don't bother at all.

Have the involved contributors any preferences on organisation
of corresponding update steps?

* Correct exception handling first and then apply the shown
  source code reduction again.

* Combine better exception handling with smaller code
  in one patch.

* Addition of the tag “Fixes” in the commit message?

Regards,
Markus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-10-31 13:33 ` Andy Shevchenko
  2017-10-31 14:24   ` SF Markus Elfring
@ 2017-11-01 18:45   ` SF Markus Elfring
  2017-11-01 18:46     ` [PATCH v2 1/3] Sony-laptop: Fix exception handling in sony_nc_setup_rfkill() SF Markus Elfring
                       ` (3 more replies)
  1 sibling, 4 replies; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-01 18:45 UTC (permalink / raw)
  To: platform-driver-x86, Andy Shevchenko, Darren Hart,
	Marco Chiappero, Matthew Garrett, Mattia Dongili
  Cc: LKML, kernel-janitors, Andy Shevchenko

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 1 Nov 2017 19:34:56 +0100

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Fix exception handling
  Delete an unnecessary variable initialisation
  Use common error handling code
---

v2:
Two additional suggestions were taken into account from a corresponding
source code review.

 drivers/platform/x86/sony-laptop.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/3] Sony-laptop: Fix exception handling in sony_nc_setup_rfkill()
  2017-11-01 18:45   ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() SF Markus Elfring
@ 2017-11-01 18:46     ` SF Markus Elfring
  2017-11-03 12:00       ` Andy Shevchenko
  2017-11-01 18:47     ` [PATCH v2 2/3] Sony-laptop: Delete an unnecessary variable initialisation " SF Markus Elfring
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-01 18:46 UTC (permalink / raw)
  To: platform-driver-x86, Andy Shevchenko, Darren Hart,
	Marco Chiappero, Matthew Garrett, Mattia Dongili
  Cc: LKML, kernel-janitors, Andy Shevchenko

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 1 Nov 2017 18:42:45 +0100

Source code review for a specific software refactoring showed the need
for another correction because the error code "-1" was returned so far
if a call of the function "sony_call_snc_handle" failed here.
Thus assign the return value from these two function calls also to
the variable "err" and provide it in case of a failure.

Fixes: d6f15ed876b83a1a0eba1d0473eef58acc95444a ("sony-laptop: use soft rfkill status stored in hw")
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Link: https://lkml.org/lkml/2017/10/31/463
Link: https://lkml.kernel.org/r/<CAHp75VcMkXCioCzmLE0+BTmkqc5RSOx9yPO0ectVHMrMvewgwg@mail.gmail.com>
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/platform/x86/sony-laptop.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index a16cea2be9c3..4332cc982ce0 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -1660,17 +1660,19 @@ static int sony_nc_setup_rfkill(struct acpi_device *device,
 	if (!rfk)
 		return -ENOMEM;
 
-	if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0) {
+	err = sony_call_snc_handle(sony_rfkill_handle, 0x200, &result);
+	if (err < 0) {
 		rfkill_destroy(rfk);
-		return -1;
+		return err;
 	}
 	hwblock = !(result & 0x1);
 
-	if (sony_call_snc_handle(sony_rfkill_handle,
-				sony_rfkill_address[nc_type],
-				&result) < 0) {
+	err = sony_call_snc_handle(sony_rfkill_handle,
+				   sony_rfkill_address[nc_type],
+				   &result);
+	if (err < 0) {
 		rfkill_destroy(rfk);
-		return -1;
+		return err;
 	}
 	swblock = !(result & 0x2);
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/3] Sony-laptop: Delete an unnecessary variable initialisation in sony_nc_setup_rfkill()
  2017-11-01 18:45   ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() SF Markus Elfring
  2017-11-01 18:46     ` [PATCH v2 1/3] Sony-laptop: Fix exception handling in sony_nc_setup_rfkill() SF Markus Elfring
@ 2017-11-01 18:47     ` SF Markus Elfring
  2017-11-03 12:00       ` Andy Shevchenko
  2017-11-01 18:49     ` [PATCH v2 3/3] Sony-laptop: Use common error handling code " SF Markus Elfring
  2017-11-03 12:02     ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() Andy Shevchenko
  3 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-01 18:47 UTC (permalink / raw)
  To: platform-driver-x86, Andy Shevchenko, Darren Hart,
	Marco Chiappero, Matthew Garrett, Mattia Dongili
  Cc: LKML, kernel-janitors, Andy Shevchenko

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 1 Nov 2017 19:00:59 +0100

The local variable "err" will eventually be set to an appropriate value
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/platform/x86/sony-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 4332cc982ce0..62aa2c37b8d2 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -1627,7 +1627,7 @@ static const struct rfkill_ops sony_rfkill_ops = {
 static int sony_nc_setup_rfkill(struct acpi_device *device,
 				enum sony_nc_rfkill nc_type)
 {
-	int err = 0;
+	int err;
 	struct rfkill *rfk;
 	enum rfkill_type type;
 	const char *name;
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/3] Sony-laptop: Use common error handling code in sony_nc_setup_rfkill()
  2017-11-01 18:45   ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() SF Markus Elfring
  2017-11-01 18:46     ` [PATCH v2 1/3] Sony-laptop: Fix exception handling in sony_nc_setup_rfkill() SF Markus Elfring
  2017-11-01 18:47     ` [PATCH v2 2/3] Sony-laptop: Delete an unnecessary variable initialisation " SF Markus Elfring
@ 2017-11-01 18:49     ` SF Markus Elfring
  2017-11-03 12:02     ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() Andy Shevchenko
  3 siblings, 0 replies; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-01 18:49 UTC (permalink / raw)
  To: platform-driver-x86, Andy Shevchenko, Darren Hart,
	Marco Chiappero, Matthew Garrett, Mattia Dongili
  Cc: LKML, kernel-janitors, Andy Shevchenko

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 1 Nov 2017 19:17:01 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

v2:
This software refactoring was repeated after the error code handling was
adjusted for two function calls.

 drivers/platform/x86/sony-laptop.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 62aa2c37b8d2..8e2ee1e221fe 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -1661,31 +1661,32 @@ static int sony_nc_setup_rfkill(struct acpi_device *device,
 		return -ENOMEM;
 
 	err = sony_call_snc_handle(sony_rfkill_handle, 0x200, &result);
-	if (err < 0) {
-		rfkill_destroy(rfk);
-		return err;
-	}
+	if (err < 0)
+		goto destroy_rfk;
+
 	hwblock = !(result & 0x1);
 
 	err = sony_call_snc_handle(sony_rfkill_handle,
 				   sony_rfkill_address[nc_type],
 				   &result);
-	if (err < 0) {
-		rfkill_destroy(rfk);
-		return err;
-	}
+	if (err < 0)
+		goto destroy_rfk;
+
 	swblock = !(result & 0x2);
 
 	rfkill_init_sw_state(rfk, swblock);
 	rfkill_set_hw_state(rfk, hwblock);
 
 	err = rfkill_register(rfk);
-	if (err) {
-		rfkill_destroy(rfk);
-		return err;
-	}
+	if (err)
+		goto destroy_rfk;
+
 	sony_rfkill_devices[nc_type] = rfk;
 	return err;
+
+destroy_rfk:
+	rfkill_destroy(rfk);
+	return err;
 }
 
 static void sony_nc_rfkill_update(void)
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/3] Sony-laptop: Fix exception handling in sony_nc_setup_rfkill()
  2017-11-01 18:46     ` [PATCH v2 1/3] Sony-laptop: Fix exception handling in sony_nc_setup_rfkill() SF Markus Elfring
@ 2017-11-03 12:00       ` Andy Shevchenko
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2017-11-03 12:00 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Platform Driver, Darren Hart, Marco Chiappero, Matthew Garrett,
	Mattia Dongili, LKML, kernel-janitors, Andy Shevchenko

On Wed, Nov 1, 2017 at 8:46 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 1 Nov 2017 18:42:45 +0100
>
> Source code review for a specific software refactoring showed the need
> for another correction because the error code "-1" was returned so far
> if a call of the function "sony_call_snc_handle" failed here.
> Thus assign the return value from these two function calls also to
> the variable "err" and provide it in case of a failure.
>

Applied to my review and testing queue, thanks!

> Fixes: d6f15ed876b83a1a0eba1d0473eef58acc95444a ("sony-laptop: use soft rfkill status stored in hw")
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Link: https://lkml.org/lkml/2017/10/31/463
> Link: https://lkml.kernel.org/r/<CAHp75VcMkXCioCzmLE0+BTmkqc5RSOx9yPO0ectVHMrMvewgwg@mail.gmail.com>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/platform/x86/sony-laptop.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
> index a16cea2be9c3..4332cc982ce0 100644
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -1660,17 +1660,19 @@ static int sony_nc_setup_rfkill(struct acpi_device *device,
>         if (!rfk)
>                 return -ENOMEM;
>
> -       if (sony_call_snc_handle(sony_rfkill_handle, 0x200, &result) < 0) {
> +       err = sony_call_snc_handle(sony_rfkill_handle, 0x200, &result);
> +       if (err < 0) {
>                 rfkill_destroy(rfk);
> -               return -1;
> +               return err;
>         }
>         hwblock = !(result & 0x1);
>
> -       if (sony_call_snc_handle(sony_rfkill_handle,
> -                               sony_rfkill_address[nc_type],
> -                               &result) < 0) {
> +       err = sony_call_snc_handle(sony_rfkill_handle,
> +                                  sony_rfkill_address[nc_type],
> +                                  &result);
> +       if (err < 0) {
>                 rfkill_destroy(rfk);
> -               return -1;
> +               return err;
>         }
>         swblock = !(result & 0x2);
>
> --
> 2.14.3
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/3] Sony-laptop: Delete an unnecessary variable initialisation in sony_nc_setup_rfkill()
  2017-11-01 18:47     ` [PATCH v2 2/3] Sony-laptop: Delete an unnecessary variable initialisation " SF Markus Elfring
@ 2017-11-03 12:00       ` Andy Shevchenko
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2017-11-03 12:00 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Platform Driver, Darren Hart, Marco Chiappero, Mattia Dongili,
	LKML, kernel-janitors, Andy Shevchenko

On Wed, Nov 1, 2017 at 8:47 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 1 Nov 2017 19:00:59 +0100
>
> The local variable "err" will eventually be set to an appropriate value
> a bit later. Thus omit the explicit initialisation at the beginning.

Applied to my review and testing queue, thanks!

>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/platform/x86/sony-laptop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
> index 4332cc982ce0..62aa2c37b8d2 100644
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -1627,7 +1627,7 @@ static const struct rfkill_ops sony_rfkill_ops = {
>  static int sony_nc_setup_rfkill(struct acpi_device *device,
>                                 enum sony_nc_rfkill nc_type)
>  {
> -       int err = 0;
> +       int err;
>         struct rfkill *rfk;
>         enum rfkill_type type;
>         const char *name;
> --
> 2.14.3
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-01 18:45   ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() SF Markus Elfring
                       ` (2 preceding siblings ...)
  2017-11-01 18:49     ` [PATCH v2 3/3] Sony-laptop: Use common error handling code " SF Markus Elfring
@ 2017-11-03 12:02     ` Andy Shevchenko
  2017-11-03 13:23       ` SF Markus Elfring
  3 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2017-11-03 12:02 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Platform Driver, Darren Hart, Marco Chiappero, Matthew Garrett,
	Mattia Dongili, LKML, kernel-janitors, Andy Shevchenko

On Wed, Nov 1, 2017 at 8:45 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 1 Nov 2017 19:34:56 +0100
>
> Three update suggestions were taken into account
> from static source code analysis.

I have applied first two, the last one is subject to discuss a necessity of it.

So, if Darren on my side than we won't apply it, if he opposes, I
would hear an argument why we might need it.

>
> Markus Elfring (3):
>   Fix exception handling
>   Delete an unnecessary variable initialisation
>   Use common error handling code
> ---
>
> v2:
> Two additional suggestions were taken into account from a corresponding
> source code review.
>
>  drivers/platform/x86/sony-laptop.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
>
> --
> 2.14.3
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-03 12:02     ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() Andy Shevchenko
@ 2017-11-03 13:23       ` SF Markus Elfring
  2017-11-05 13:24         ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-03 13:23 UTC (permalink / raw)
  To: Andy Shevchenko, Darren Hart, platform-driver-x86
  Cc: Marco Chiappero, Matthew Garrett, Mattia Dongili, LKML,
	kernel-janitors, Andy Shevchenko

> I have applied first two,

Thanks for another change acceptance.


> the last one is subject to discuss a necessity of it.

I can offer another bit of information for this software development discussion.

The following build settings were active in my “Makefile” for this Linux test case.

…
HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O0 -fomit-frame-pointer -std=gnu89
…


The affected source file can be compiled for the processor architecture “x86_64”
by a tool like “GCC 6.4.1+r251631-1.3” from the software distribution
“openSUSE Tumbleweed” with the following command example.

my_cc=/usr/bin/gcc-6 \
&& my_module=drivers/platform/x86/sony-laptop.ko \
&& git checkout ':/^Sony-laptop: Delete an unnecessary variable initialisation in sony_nc_setup_rfkill' \
&& make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
&& size "${my_module}" \
&& git checkout ':/^Sony-laptop: Use common error handling code in sony_nc_setup_rfkill' \
&& make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
&& size "${my_module}"


Do you find the following details useful for further clarification?

text: -32
data: 0
bss:  0

Regards,
Markus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-03 13:23       ` SF Markus Elfring
@ 2017-11-05 13:24         ` Andy Shevchenko
  2017-11-05 14:20           ` SF Markus Elfring
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2017-11-05 13:24 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Darren Hart, Platform Driver, Marco Chiappero, Matthew Garrett,
	Mattia Dongili, LKML, kernel-janitors, Andy Shevchenko

On Fri, Nov 3, 2017 at 3:23 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> I have applied first two,
>
> Thanks for another change acceptance.
>
>
>> the last one is subject to discuss a necessity of it.
>
> I can offer another bit of information for this software development discussion.
>
> The following build settings were active in my “Makefile” for this Linux test case.
>
> …
> HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O0 -fomit-frame-pointer -std=gnu89
> …
>
>
> The affected source file can be compiled for the processor architecture “x86_64”
> by a tool like “GCC 6.4.1+r251631-1.3” from the software distribution
> “openSUSE Tumbleweed” with the following command example.
>
> my_cc=/usr/bin/gcc-6 \
> && my_module=drivers/platform/x86/sony-laptop.ko \
> && git checkout ':/^Sony-laptop: Delete an unnecessary variable initialisation in sony_nc_setup_rfkill' \
> && make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
> && size "${my_module}" \
> && git checkout ':/^Sony-laptop: Use common error handling code in sony_nc_setup_rfkill' \
> && make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
> && size "${my_module}"
>
>
> Do you find the following details useful for further clarification?
>
> text: -32
> data: 0
> bss:  0

...but kernel is compiled with -O2 which, I suppose, will eliminate
these repeats.

So, the main question is "WHY" you are doing this change.

I didn't find any convinced explanation (yet?).

As an example, I would understand it if the consequent patch will
bring locking to the function.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-05 13:24         ` Andy Shevchenko
@ 2017-11-05 14:20           ` SF Markus Elfring
  2017-11-07 12:49             ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-05 14:20 UTC (permalink / raw)
  To: Andy Shevchenko, platform-driver-x86
  Cc: Darren Hart, Marco Chiappero, Matthew Garrett, Mattia Dongili,
	LKML, kernel-janitors, Andy Shevchenko

>> Do you find the following details useful for further clarification?
>>
>> text: -32
>> data: 0
>> bss:  0
> 
> ...but kernel is compiled with -O2 which,

Did you try to compare results for my update suggestion also with
the default build configuration?


> I suppose, will eliminate these repeats.

My build results did not fit to your expectations.


> So, the main question is "WHY" you are doing this change.
> 
> I didn't find any convinced explanation (yet?).

Do you find a reduction of 32 bytes for the object code not interesting
enough just by achieving such a small effect because of the replacement
of three function calls “rfkill_destroy” with goto statements?

Regards,
Markus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-05 14:20           ` SF Markus Elfring
@ 2017-11-07 12:49             ` Andy Shevchenko
  2017-11-07 13:00               ` SF Markus Elfring
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2017-11-07 12:49 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Platform Driver, Darren Hart, Marco Chiappero, Matthew Garrett,
	Mattia Dongili, LKML, kernel-janitors, Andy Shevchenko

On Sun, Nov 5, 2017 at 4:20 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>>> Do you find the following details useful for further clarification?
>>>
>>> text: -32
>>> data: 0
>>> bss:  0

...

>> So, the main question is "WHY" you are doing this change.
>>
>> I didn't find any convinced explanation (yet?).
>
> Do you find a reduction of 32 bytes for the object code not interesting
> enough just by achieving such a small effect because of the replacement
> of three function calls “rfkill_destroy” with goto statements?

Okay, it's something.

Update a commit message in your patch, use bloat-o-meter (and put its
output to the commit message) and resend, we might reconsider then.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-07 12:49             ` Andy Shevchenko
@ 2017-11-07 13:00               ` SF Markus Elfring
  2017-11-07 13:18                 ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-07 13:00 UTC (permalink / raw)
  To: Andy Shevchenko, platform-driver-x86
  Cc: Darren Hart, Marco Chiappero, Mattia Dongili, LKML,
	kernel-janitors, Andy Shevchenko

> Update a commit message in your patch, use bloat-o-meter (and put its
> output to the commit message) and resend, we might reconsider then.

Do you expect a resend only for the third update step of this small
patch series (together with additional information)?

Regards,
Markus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-07 13:00               ` SF Markus Elfring
@ 2017-11-07 13:18                 ` Andy Shevchenko
  2017-11-07 13:54                   ` SF Markus Elfring
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2017-11-07 13:18 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Platform Driver, Darren Hart, Marco Chiappero, Mattia Dongili,
	LKML, kernel-janitors, Andy Shevchenko

On Tue, Nov 7, 2017 at 3:00 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> Update a commit message in your patch, use bloat-o-meter (and put its
>> output to the commit message) and resend, we might reconsider then.
>
> Do you expect a resend only for the third update step of this small
> patch series (together with additional information)?

First two already applied.
So, the third patch is in question, please resend it with asked commit
message update.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Sony-laptop: Adjustments for sony_nc_setup_rfkill()
  2017-11-07 13:18                 ` Andy Shevchenko
@ 2017-11-07 13:54                   ` SF Markus Elfring
  0 siblings, 0 replies; 19+ messages in thread
From: SF Markus Elfring @ 2017-11-07 13:54 UTC (permalink / raw)
  To: Andy Shevchenko, platform-driver-x86
  Cc: Darren Hart, Marco Chiappero, Mattia Dongili, LKML,
	kernel-janitors, Andy Shevchenko

> First two already applied.

This is nice.


> So, the third patch is in question, please resend it with asked commit
> message update.

I find such a resend questionable at the moment.

Which details do you expect from the software “scripts/bloat-o-meter”
more than I provided for the discussed Linux module already?

Regards,
Markus

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-11-07 13:55 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-30 20:15 [PATCH] Sony-laptop: Use common error handling code in sony_nc_setup_rfkill() SF Markus Elfring
2017-10-31 13:33 ` Andy Shevchenko
2017-10-31 14:24   ` SF Markus Elfring
2017-10-31 16:35     ` Andy Shevchenko
2017-10-31 16:56       ` SF Markus Elfring
2017-11-01 18:45   ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() SF Markus Elfring
2017-11-01 18:46     ` [PATCH v2 1/3] Sony-laptop: Fix exception handling in sony_nc_setup_rfkill() SF Markus Elfring
2017-11-03 12:00       ` Andy Shevchenko
2017-11-01 18:47     ` [PATCH v2 2/3] Sony-laptop: Delete an unnecessary variable initialisation " SF Markus Elfring
2017-11-03 12:00       ` Andy Shevchenko
2017-11-01 18:49     ` [PATCH v2 3/3] Sony-laptop: Use common error handling code " SF Markus Elfring
2017-11-03 12:02     ` [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill() Andy Shevchenko
2017-11-03 13:23       ` SF Markus Elfring
2017-11-05 13:24         ` Andy Shevchenko
2017-11-05 14:20           ` SF Markus Elfring
2017-11-07 12:49             ` Andy Shevchenko
2017-11-07 13:00               ` SF Markus Elfring
2017-11-07 13:18                 ` Andy Shevchenko
2017-11-07 13:54                   ` SF Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).