[PATCH] staging: vt6656: Remove unneccesary return variable

[PATCH] staging: vt6656: Remove unneccesary return variable

[Namrata A Shettar]

Remove uneccessary return variable 'ret'. Return value directly.

Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
---
 drivers/staging/vt6656/card.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 53b469c..33de99a 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -708,8 +708,6 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
  */
 int vnt_radio_power_off(struct vnt_private *priv)
 {
-	int ret = true;
-
 	switch (priv->rf_type) {
 	case RF_AL2230:
 	case RF_AL2230S:
@@ -728,7 +726,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
 
 	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
 
-	return ret;
+	return true;
 }
 
 /*
@@ -745,8 +743,6 @@ int vnt_radio_power_off(struct vnt_private *priv)
  */
 int vnt_radio_power_on(struct vnt_private *priv)
 {
-	int ret = true;
-
 	vnt_exit_deep_sleep(priv);
 
 	vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
@@ -765,7 +761,7 @@ int vnt_radio_power_on(struct vnt_private *priv)
 
 	vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
 
-	return ret;
+	return true;
 }
 
 void vnt_set_bss_mode(struct vnt_private *priv)
-- 
2.7.4

Re: [PATCH] staging: vt6656: Remove unneccesary return variable

[Greg Kroah-Hartman]

On Mon, Sep 26, 2016 at 04:42:03PM +0530, Namrata A Shettar wrote:
> Remove uneccessary return variable 'ret'. Return value directly.
> 
> Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
> ---
>  drivers/staging/vt6656/card.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> index 53b469c..33de99a 100644
> --- a/drivers/staging/vt6656/card.c
> +++ b/drivers/staging/vt6656/card.c
> @@ -708,8 +708,6 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
>   */
>  int vnt_radio_power_off(struct vnt_private *priv)
>  {
> -	int ret = true;
> -
>  	switch (priv->rf_type) {
>  	case RF_AL2230:
>  	case RF_AL2230S:
> @@ -728,7 +726,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
>  
>  	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
>  
> -	return ret;
> +	return true;
>  }

If this function can never "fail", why return anything at all?  Just
make the function not return anything, it's not like the return value is
even checked.

>  
>  /*
> @@ -745,8 +743,6 @@ int vnt_radio_power_off(struct vnt_private *priv)
>   */
>  int vnt_radio_power_on(struct vnt_private *priv)
>  {
> -	int ret = true;
> -
>  	vnt_exit_deep_sleep(priv);
>  
>  	vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
> @@ -765,7 +761,7 @@ int vnt_radio_power_on(struct vnt_private *priv)
>  
>  	vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
>  
> -	return ret;
> +	return true;
>  }
i
Same here.

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH] staging: vt6656: Remove unneccesary return variable

[Julia Lawall]

On Mon, 26 Sep 2016, Greg Kroah-Hartman wrote:

> On Mon, Sep 26, 2016 at 04:42:03PM +0530, Namrata A Shettar wrote:
> > Remove uneccessary return variable 'ret'. Return value directly.
> >
> > Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
> > ---
> >  drivers/staging/vt6656/card.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> > index 53b469c..33de99a 100644
> > --- a/drivers/staging/vt6656/card.c
> > +++ b/drivers/staging/vt6656/card.c
> > @@ -708,8 +708,6 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
> >   */
> >  int vnt_radio_power_off(struct vnt_private *priv)
> >  {
> > -	int ret = true;
> > -
> >  	switch (priv->rf_type) {
> >  	case RF_AL2230:
> >  	case RF_AL2230S:
> > @@ -728,7 +726,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
> >
> >  	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> >
> > -	return ret;
> > +	return true;

One could also wonder why the return type is int and not bool, but if no
one cares about the value then it is certainly better to just get rid of
it.

julia

> >  }
>
> If this function can never "fail", why return anything at all?  Just
> make the function not return anything, it's not like the return value is
> even checked.
>
> >
> >  /*
> > @@ -745,8 +743,6 @@ int vnt_radio_power_off(struct vnt_private *priv)
> >   */
> >  int vnt_radio_power_on(struct vnt_private *priv)
> >  {
> > -	int ret = true;
> > -
> >  	vnt_exit_deep_sleep(priv);
> >
> >  	vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
> > @@ -765,7 +761,7 @@ int vnt_radio_power_on(struct vnt_private *priv)
> >
> >  	vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> >
> > -	return ret;
> > +	return true;
> >  }
> i
> Same here.
>
> thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160926112353.GA4505%40kroah.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH] staging: vt6656: Remove unneccesary return variable

[Namrata A Shettar]

On Mon, Sep 26, 2016 at 01:26:47PM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 26 Sep 2016, Greg Kroah-Hartman wrote:
> 
> > On Mon, Sep 26, 2016 at 04:42:03PM +0530, Namrata A Shettar wrote:
> > > Remove uneccessary return variable 'ret'. Return value directly.
> > >
> > > Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
> > > ---
> > >  drivers/staging/vt6656/card.c | 8 ++------
> > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> > > index 53b469c..33de99a 100644
> > > --- a/drivers/staging/vt6656/card.c
> > > +++ b/drivers/staging/vt6656/card.c
> > > @@ -708,8 +708,6 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
> > >   */
> > >  int vnt_radio_power_off(struct vnt_private *priv)
> > >  {
> > > -	int ret = true;
> > > -
> > >  	switch (priv->rf_type) {
> > >  	case RF_AL2230:
> > >  	case RF_AL2230S:
> > > @@ -728,7 +726,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > >
> > >  	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > >
> > > -	return ret;
> > > +	return true;
> 
> One could also wonder why the return type is int and not bool, but if no
> one cares about the value then it is certainly better to just get rid of
> it.
> 
> julia

I should have changed the return type to 'bool' when I eliminated the
'ret' variable. But I found that returning 'true' or 'false' with return
type 'int' works well too. I would have to change it if I weren't dropping
the return value I suppose.

> 
> > >  }
> >
> > If this function can never "fail", why return anything at all?  Just
> > make the function not return anything, it's not like the return value is
> > even checked.

The description given in the comments for the two functions suggest that
return value be 'true' if success else 'false'. I grepped once to check
if the return value is used anywhere..it isn't, so I'll remove the
return value along with the comment on the return value?

Also, I found another another function 'vnt_get_current_tsf' whose return
value isn't checked anywhere, so can I safely remove the return value
from that function too?

thanks,

namrata

> >
> > >
> > >  /*
> > > @@ -745,8 +743,6 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > >   */
> > >  int vnt_radio_power_on(struct vnt_private *priv)
> > >  {
> > > -	int ret = true;
> > > -
> > >  	vnt_exit_deep_sleep(priv);
> > >
> > >  	vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
> > > @@ -765,7 +761,7 @@ int vnt_radio_power_on(struct vnt_private *priv)
> > >
> > >  	vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > >
> > > -	return ret;
> > > +	return true;
> > >  }
> > i
> > Same here.
> >
> > thanks,
> >
> > greg k-h
> >

Re: [Outreachy kernel] Re: [PATCH] staging: vt6656: Remove unneccesary return variable

[Julia Lawall]

On Mon, 26 Sep 2016, Namrata A Shettar wrote:

> On Mon, Sep 26, 2016 at 01:26:47PM +0200, Julia Lawall wrote:
> >
> >
> > On Mon, 26 Sep 2016, Greg Kroah-Hartman wrote:
> >
> > > On Mon, Sep 26, 2016 at 04:42:03PM +0530, Namrata A Shettar wrote:
> > > > Remove uneccessary return variable 'ret'. Return value directly.
> > > >
> > > > Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
> > > > ---
> > > >  drivers/staging/vt6656/card.c | 8 ++------
> > > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> > > > index 53b469c..33de99a 100644
> > > > --- a/drivers/staging/vt6656/card.c
> > > > +++ b/drivers/staging/vt6656/card.c
> > > > @@ -708,8 +708,6 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
> > > >   */
> > > >  int vnt_radio_power_off(struct vnt_private *priv)
> > > >  {
> > > > -	int ret = true;
> > > > -
> > > >  	switch (priv->rf_type) {
> > > >  	case RF_AL2230:
> > > >  	case RF_AL2230S:
> > > > @@ -728,7 +726,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > > >
> > > >  	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > > >
> > > > -	return ret;
> > > > +	return true;
> >
> > One could also wonder why the return type is int and not bool, but if no
> > one cares about the value then it is certainly better to just get rid of
> > it.
> >
> > julia
>
> I should have changed the return type to 'bool' when I eliminated the
> 'ret' variable. But I found that returning 'true' or 'false' with return
> type 'int' works well too. I would have to change it if I weren't dropping
> the return value I suppose.
>
> >
> > > >  }
> > >
> > > If this function can never "fail", why return anything at all?  Just
> > > make the function not return anything, it's not like the return value is
> > > even checked.
>
> The description given in the comments for the two functions suggest that
> return value be 'true' if success else 'false'. I grepped once to check
> if the return value is used anywhere..it isn't, so I'll remove the
> return value along with the comment on the return value?
>
> Also, I found another another function 'vnt_get_current_tsf' whose return
> value isn't checked anywhere, so can I safely remove the return value
> from that function too?

As a general principle, it should be fine.  The only issue is to be sure
that the function is not stored in a structure field that has some
particular type.  The point is just that you have to check all of the
references, and not just teh references that have the form of a function
call.

julia

>
> thanks,
>
> namrata
>
> > >
> > > >
> > > >  /*
> > > > @@ -745,8 +743,6 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > > >   */
> > > >  int vnt_radio_power_on(struct vnt_private *priv)
> > > >  {
> > > > -	int ret = true;
> > > > -
> > > >  	vnt_exit_deep_sleep(priv);
> > > >
> > > >  	vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
> > > > @@ -765,7 +761,7 @@ int vnt_radio_power_on(struct vnt_private *priv)
> > > >
> > > >  	vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > > >
> > > > -	return ret;
> > > > +	return true;
> > > >  }
> > > i
> > > Same here.
> > >
> > > thanks,
> > >
> > > greg k-h
> > >
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160926122923.GA18805%40namrata-HP-Pavilion-g6-Notebook-PC.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH] staging: vt6656: Remove unneccesary return variable

[Namrata A Shettar]

On Mon, Sep 26, 2016 at 04:31:11PM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 26 Sep 2016, Namrata A Shettar wrote:
> 
> > On Mon, Sep 26, 2016 at 01:26:47PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Mon, 26 Sep 2016, Greg Kroah-Hartman wrote:
> > >
> > > > On Mon, Sep 26, 2016 at 04:42:03PM +0530, Namrata A Shettar wrote:
> > > > > Remove uneccessary return variable 'ret'. Return value directly.
> > > > >
> > > > > Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
> > > > > ---
> > > > >  drivers/staging/vt6656/card.c | 8 ++------
> > > > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> > > > > index 53b469c..33de99a 100644
> > > > > --- a/drivers/staging/vt6656/card.c
> > > > > +++ b/drivers/staging/vt6656/card.c
> > > > > @@ -708,8 +708,6 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
> > > > >   */
> > > > >  int vnt_radio_power_off(struct vnt_private *priv)
> > > > >  {
> > > > > -	int ret = true;
> > > > > -
> > > > >  	switch (priv->rf_type) {
> > > > >  	case RF_AL2230:
> > > > >  	case RF_AL2230S:
> > > > > @@ -728,7 +726,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > > > >
> > > > >  	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > > > >
> > > > > -	return ret;
> > > > > +	return true;
> > >
> > > One could also wonder why the return type is int and not bool, but if no
> > > one cares about the value then it is certainly better to just get rid of
> > > it.
> > >
> > > julia
> >
> > I should have changed the return type to 'bool' when I eliminated the
> > 'ret' variable. But I found that returning 'true' or 'false' with return
> > type 'int' works well too. I would have to change it if I weren't dropping
> > the return value I suppose.
> >
> > >
> > > > >  }
> > > >
> > > > If this function can never "fail", why return anything at all?  Just
> > > > make the function not return anything, it's not like the return value is
> > > > even checked.
> >
> > The description given in the comments for the two functions suggest that
> > return value be 'true' if success else 'false'. I grepped once to check
> > if the return value is used anywhere..it isn't, so I'll remove the
> > return value along with the comment on the return value?
> >
> > Also, I found another another function 'vnt_get_current_tsf' whose return
> > value isn't checked anywhere, so can I safely remove the return value
> > from that function too?
> 
> As a general principle, it should be fine.  The only issue is to be sure
> that the function is not stored in a structure field that has some
> particular type.  The point is just that you have to check all of the
> references, and not just teh references that have the form of a function
> call.
> 
> julia
> 

Alright. So I'll remove the return values of all these functions and the
comments and resend the patch?. Hope that is what was meant.

thanks,

namrata
> >
> > > >
> > > > >
> > > > >  /*
> > > > > @@ -745,8 +743,6 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > > > >   */
> > > > >  int vnt_radio_power_on(struct vnt_private *priv)
> > > > >  {
> > > > > -	int ret = true;
> > > > > -
> > > > >  	vnt_exit_deep_sleep(priv);
> > > > >
> > > > >  	vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
> > > > > @@ -765,7 +761,7 @@ int vnt_radio_power_on(struct vnt_private *priv)
> > > > >
> > > > >  	vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > > > >
> > > > > -	return ret;
> > > > > +	return true;
> > > > >  }
> > > > i
> > > > Same here.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h

Re: [Outreachy kernel] Re: [PATCH] staging: vt6656: Remove unneccesary return variable

[Julia Lawall]

On Mon, 26 Sep 2016, Namrata A Shettar wrote:

> On Mon, Sep 26, 2016 at 04:31:11PM +0200, Julia Lawall wrote:
> >
> >
> > On Mon, 26 Sep 2016, Namrata A Shettar wrote:
> >
> > > On Mon, Sep 26, 2016 at 01:26:47PM +0200, Julia Lawall wrote:
> > > >
> > > >
> > > > On Mon, 26 Sep 2016, Greg Kroah-Hartman wrote:
> > > >
> > > > > On Mon, Sep 26, 2016 at 04:42:03PM +0530, Namrata A Shettar wrote:
> > > > > > Remove uneccessary return variable 'ret'. Return value directly.
> > > > > >
> > > > > > Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
> > > > > > ---
> > > > > >  drivers/staging/vt6656/card.c | 8 ++------
> > > > > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> > > > > > index 53b469c..33de99a 100644
> > > > > > --- a/drivers/staging/vt6656/card.c
> > > > > > +++ b/drivers/staging/vt6656/card.c
> > > > > > @@ -708,8 +708,6 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
> > > > > >   */
> > > > > >  int vnt_radio_power_off(struct vnt_private *priv)
> > > > > >  {
> > > > > > -	int ret = true;
> > > > > > -
> > > > > >  	switch (priv->rf_type) {
> > > > > >  	case RF_AL2230:
> > > > > >  	case RF_AL2230S:
> > > > > > @@ -728,7 +726,7 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > > > > >
> > > > > >  	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > > > > >
> > > > > > -	return ret;
> > > > > > +	return true;
> > > >
> > > > One could also wonder why the return type is int and not bool, but if no
> > > > one cares about the value then it is certainly better to just get rid of
> > > > it.
> > > >
> > > > julia
> > >
> > > I should have changed the return type to 'bool' when I eliminated the
> > > 'ret' variable. But I found that returning 'true' or 'false' with return
> > > type 'int' works well too. I would have to change it if I weren't dropping
> > > the return value I suppose.
> > >
> > > >
> > > > > >  }
> > > > >
> > > > > If this function can never "fail", why return anything at all?  Just
> > > > > make the function not return anything, it's not like the return value is
> > > > > even checked.
> > >
> > > The description given in the comments for the two functions suggest that
> > > return value be 'true' if success else 'false'. I grepped once to check
> > > if the return value is used anywhere..it isn't, so I'll remove the
> > > return value along with the comment on the return value?
> > >
> > > Also, I found another another function 'vnt_get_current_tsf' whose return
> > > value isn't checked anywhere, so can I safely remove the return value
> > > from that function too?
> >
> > As a general principle, it should be fine.  The only issue is to be sure
> > that the function is not stored in a structure field that has some
> > particular type.  The point is just that you have to check all of the
> > references, and not just teh references that have the form of a function
> > call.
> >
> > julia
> >
>
> Alright. So I'll remove the return values of all these functions and the
> comments and resend the patch?. Hope that is what was meant.

Seems reasonable.

julia


>
> thanks,
>
> namrata
> > >
> > > > >
> > > > > >
> > > > > >  /*
> > > > > > @@ -745,8 +743,6 @@ int vnt_radio_power_off(struct vnt_private *priv)
> > > > > >   */
> > > > > >  int vnt_radio_power_on(struct vnt_private *priv)
> > > > > >  {
> > > > > > -	int ret = true;
> > > > > > -
> > > > > >  	vnt_exit_deep_sleep(priv);
> > > > > >
> > > > > >  	vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
> > > > > > @@ -765,7 +761,7 @@ int vnt_radio_power_on(struct vnt_private *priv)
> > > > > >
> > > > > >  	vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
> > > > > >
> > > > > > -	return ret;
> > > > > > +	return true;
> > > > > >  }
> > > > > i
> > > > > Same here.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160926162818.GA4292%40namrata-HP-Pavilion-g6-Notebook-PC.
> For more options, visit https://groups.google.com/d/optout.
>