[PATCH v2 05/10] staging: ks7010: Delete unnecessary uses of the variable "retval"

From: Markus Elfring Date: Wed, 10 Aug 2016 18:56:31 +0200 A few return values can also be directly used for condition checks. Thus remove a local variable for intermediate assignments. Signed-off-by: Markus Elfring --- v2: Rebased

[PATCH v2 05/10] staging: ks7010: Delete unnecessary uses of the variable "retval"

From: Markus Elfring Date: Wed, 10 Aug 2016 18:56:31 +0200 A few return values can also be directly used for condition checks. Thus remove a local variable for intermediate assignments. Signed-off-by: Markus Elfring --- v2: Rebased on the source files from Linux next-20160809. Touched

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> Do you find such a source code transformation useful? No. I already gave that answer by agreeing to Julia's viewpoint. signature.asc Description: PGP signature

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> Do you find such a source code transformation useful? No. I already gave that answer by agreeing to Julia's viewpoint. signature.asc Description: PGP signature

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

>> Can you follow expectations around the proposed refactoring of any >> function implementations? > > I don't understand both questions. Maybe you need to give examples? I suggest to try the following script (semantic patch for working with the Coccinelle software) out on the discussed source

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

>> Can you follow expectations around the proposed refactoring of any >> function implementations? > > I don't understand both questions. Maybe you need to give examples? I suggest to try the following script (semantic patch for working with the Coccinelle software) out on the discussed source

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, Jul 21, 2016 at 03:35:03PM +0200, SF Markus Elfring wrote: > > if (atomic_read(>sleepstatus.status) == 0) { > > rw_data = GCR_B_DOZE; > > - retval = > > - ks7010_sdio_write(priv, GCR_B, _data, > >

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, Jul 21, 2016 at 03:35:03PM +0200, SF Markus Elfring wrote: > > if (atomic_read(>sleepstatus.status) == 0) { > > rw_data = GCR_B_DOZE; > > - retval = > > - ks7010_sdio_write(priv, GCR_B, _data, > >

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> > * Do you really want to assign every return value from a function call > > to an extra variable before it is used again? > > If it's a choice between 1) function call on many lines, 2) function call > over 80 characters, and 3) extra variable, I definitely prefer the extra > variable. +1

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> > * Do you really want to assign every return value from a function call > > to an extra variable before it is used again? > > If it's a choice between 1) function call on many lines, 2) function call > over 80 characters, and 3) extra variable, I definitely prefer the extra > variable. +1

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> if (atomic_read(>sleepstatus.status) == 0) { > rw_data = GCR_B_DOZE; > - retval = > - ks7010_sdio_write(priv, GCR_B, _data, sizeof(rw_data)); > - if (retval) { > + if (ks7010_sdio_write(priv, > +

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> if (atomic_read(>sleepstatus.status) == 0) { > rw_data = GCR_B_DOZE; > - retval = > - ks7010_sdio_write(priv, GCR_B, _data, sizeof(rw_data)); > - if (retval) { > + if (ks7010_sdio_write(priv, > +

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, 21 Jul 2016, SF Markus Elfring wrote: > > I think the original code was fine. > > I suggest to reconsider involved implementation details once more. > > > > x = blah(); if (x) ... is a perfectly familiar kernel coding pattern. > > I can agree to such a general information. > > > > There

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, 21 Jul 2016, SF Markus Elfring wrote: > > I think the original code was fine. > > I suggest to reconsider involved implementation details once more. > > > > x = blah(); if (x) ... is a perfectly familiar kernel coding pattern. > > I can agree to such a general information. > > > > There

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> I think the original code was fine. I suggest to reconsider involved implementation details once more. > x = blah(); if (x) ... is a perfectly familiar kernel coding pattern. I can agree to such a general information. > There is no benefit in terms of performance It might be possible that

Re: staging: ks7010: Delete unnecessary uses of the variable "retval"

> I think the original code was fine. I suggest to reconsider involved implementation details once more. > x = blah(); if (x) ... is a perfectly familiar kernel coding pattern. I can agree to such a general information. > There is no benefit in terms of performance It might be possible that

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, Jul 21, 2016 at 09:26:33AM +0200, Julia Lawall wrote: > > > On Thu, 21 Jul 2016, Wolfram Sang wrote: > > > > > > >>> if (atomic_read(>sleepstatus.status) == 0) { > > > >>> rw_data = GCR_B_DOZE; > > > >>> - retval = > > > >>> -

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, Jul 21, 2016 at 09:26:33AM +0200, Julia Lawall wrote: > > > On Thu, 21 Jul 2016, Wolfram Sang wrote: > > > > > > >>> if (atomic_read(>sleepstatus.status) == 0) { > > > >>> rw_data = GCR_B_DOZE; > > > >>> - retval = > > > >>> -

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, 21 Jul 2016, Wolfram Sang wrote: > > > >>> if (atomic_read(>sleepstatus.status) == 0) { > > >>> rw_data = GCR_B_DOZE; > > >>> - retval = > > >>> - ks7010_sdio_write(priv, GCR_B, _data, > > >>> sizeof(rw_data)); > > >>> -

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Thu, 21 Jul 2016, Wolfram Sang wrote: > > > >>> if (atomic_read(>sleepstatus.status) == 0) { > > >>> rw_data = GCR_B_DOZE; > > >>> - retval = > > >>> - ks7010_sdio_write(priv, GCR_B, _data, > > >>> sizeof(rw_data)); > > >>> -

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

> >>> if (atomic_read(>sleepstatus.status) == 0) { > >>> rw_data = GCR_B_DOZE; > >>> - retval = > >>> - ks7010_sdio_write(priv, GCR_B, _data, sizeof(rw_data)); > >>> - if (retval) { > >>> + if (ks7010_sdio_write(priv, > >>> +

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

> >>> if (atomic_read(>sleepstatus.status) == 0) { > >>> rw_data = GCR_B_DOZE; > >>> - retval = > >>> - ks7010_sdio_write(priv, GCR_B, _data, sizeof(rw_data)); > >>> - if (retval) { > >>> + if (ks7010_sdio_write(priv, > >>> +

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

>>> @@ -90,7 +90,6 @@ static int ks7010_sdio_write(struct ks_wlan_private >>> *priv, unsigned int address, >>> void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv) >>> { >>> unsigned char rw_data; >>> - int retval; >>> >>> DPRINTK(4, "\n"); >>> >>> @@ -99,9 +98,10 @@ void

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

>>> @@ -90,7 +90,6 @@ static int ks7010_sdio_write(struct ks_wlan_private >>> *priv, unsigned int address, >>> void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv) >>> { >>> unsigned char rw_data; >>> - int retval; >>> >>> DPRINTK(4, "\n"); >>> >>> @@ -99,9 +98,10 @@ void

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Sun, Jul 17, 2016 at 08:56:59PM +0200, Julia Lawall wrote: > > > On Sun, 17 Jul 2016, SF Markus Elfring wrote: > > > From: Markus Elfring > > Date: Sun, 17 Jul 2016 18:15:23 +0200 > > > > Some return values can also be directly used for various condition

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Sun, Jul 17, 2016 at 08:56:59PM +0200, Julia Lawall wrote: > > > On Sun, 17 Jul 2016, SF Markus Elfring wrote: > > > From: Markus Elfring > > Date: Sun, 17 Jul 2016 18:15:23 +0200 > > > > Some return values can also be directly used for various condition checks. > > Thus remove a local

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Sun, 17 Jul 2016, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sun, 17 Jul 2016 18:15:23 +0200 > > Some return values can also be directly used for various condition checks. > Thus remove a local variable for intermediate assignments. > >

Re: [PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

On Sun, 17 Jul 2016, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sun, 17 Jul 2016 18:15:23 +0200 > > Some return values can also be directly used for various condition checks. > Thus remove a local variable for intermediate assignments. > > Signed-off-by: Markus Elfring > --- >

[PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

From: Markus Elfring Date: Sun, 17 Jul 2016 18:15:23 +0200 Some return values can also be directly used for various condition checks. Thus remove a local variable for intermediate assignments. Signed-off-by: Markus Elfring ---

[PATCH 5/9] staging: ks7010: Delete unnecessary uses of the variable "retval"

From: Markus Elfring Date: Sun, 17 Jul 2016 18:15:23 +0200 Some return values can also be directly used for various condition checks. Thus remove a local variable for intermediate assignments. Signed-off-by: Markus Elfring --- drivers/staging/ks7010/ks7010_sdio.c | 81