[PATCH 1/5] staging: lustre: Remove unnecessary else after return

[PATCH 1/5] staging: lustre: Remove unnecessary else after return

[simran singhal]

This patch fixes the checkpatch warning that else is not generally
useful after a break or return.

@@
expression e2;
statement s1;
@@
if(e2) { ... return ...; }
-else
         s1

Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 3 +--
 drivers/staging/lustre/lustre/ldlm/ldlm_pool.c      | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
index fbbd8a5..02d49b7 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
@@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct lnet_process_id id, __u32 ipaddr)
 
 	if (!count)
 		return -ENOENT;
-	else
-		return 0;
+	return 0;
 }
 
 void
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
index cf3fc57..ac32c82 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
@@ -338,8 +338,7 @@ static int ldlm_cli_pool_shrink(struct ldlm_pool *pl,
 
 	if (nr = 0)
 		return (unused / 100) * sysctl_vfs_cache_pressure;
-	else
-		return ldlm_cancel_lru(ns, nr, LCF_ASYNC, LDLM_LRU_FLAG_SHRINK);
+	return ldlm_cancel_lru(ns, nr, LCF_ASYNC, LDLM_LRU_FLAG_SHRINK);
 }
 
 static const struct ldlm_pool_ops ldlm_cli_pool_ops = {
-- 
2.7.4

[PATCH 2/5] staging: rtl8192u: Remove unnecessary else after return

[simran singhal]

This patch fixes the checkpatch warning that else is not generally
useful after a break or return.

This was done using Coccinelle:

@@
expression e2;
statement s1;
@@
if(e2) { ... return ...; }
-else
         s1

Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 2453413..4d6c928 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -374,8 +374,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 
 	if (!tcb_desc->bHwSec)
 		return ret;
-	else
-		return 0;
+	return 0;
 
 
 }
-- 
2.7.4

Re: [Outreachy kernel] [PATCH 2/5] staging: rtl8192u: Remove unnecessary else after return

[Julia Lawall]

On Mon, 27 Feb 2017, simran singhal wrote:

> This patch fixes the checkpatch warning that else is not generally
> useful after a break or return.
>
> This was done using Coccinelle:
>
> @@
> expression e2;
> statement s1;
> @@
> if(e2) { ... return ...; }
> -else
>          s1
>
> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> ---
>  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
> index 2453413..4d6c928 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
> @@ -374,8 +374,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
>
>  	if (!tcb_desc->bHwSec)
>  		return ret;
> -	else
> -		return 0;
> +	return 0;

In contrast to another patch I commented on, it seems likely that here 0
means success.  Converting 0 to false when that is what it means (ie not
here) makes the code more understandable.

julia

[PATCH 3/5] staging: rtl8712: Remove unnecessary else after return

[simran singhal]

This patch fixes the checkpatch warning that else is not generally
useful after a break or return.

This was done using Coccinelle:
@@
expression e2;
statement s1;
@@
if(e2) { ... return ...; }
-else
         s1

Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
 drivers/staging/rtl8712/os_intfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index 8836b31..3062167 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -411,8 +411,7 @@ static int netdev_open(struct net_device *pnetdev)
 			goto netdev_open_error;
 		if (!padapter->dvobjpriv.inirp_init)
 			goto netdev_open_error;
-		else
-			padapter->dvobjpriv.inirp_init(padapter);
+		padapter->dvobjpriv.inirp_init(padapter);
 		r8712_set_ps_mode(padapter, padapter->registrypriv.power_mgnt,
 				  padapter->registrypriv.smart_ps);
 	}
-- 
2.7.4

Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return

[Julia Lawall]

On Mon, 27 Feb 2017, simran singhal wrote:

> This patch fixes the checkpatch warning that else is not generally
> useful after a break or return.
>
> This was done using Coccinelle:
> @@
> expression e2;
> statement s1;
> @@
> if(e2) { ... return ...; }
> -else
>          s1

One might be surprised that the following code was detected using the
above semantic patch, because in the code below there is no return in the
if branches.  Actually, as a special feature, when one has an if branch
that ends in return, Coccinelle will skip through any gotos and see if the
return is matched afterward.  Indeed it is a common pattern to have

if (...) {
   foo(x);
   bar(y);
   return -ENOMEM;
}

But the code can also be cut up as eg

if (...) {
   ret = -ENOMEM;
   goto out;
}
...
out:
   foo(x);
   bar(y);
   return ret;

To avoid having to write multiple patterns for these cases, Coccinelle
will just jump through the return in the second case, allowing the same
pattern to match both of them.

julia

>
> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> ---
>  drivers/staging/rtl8712/os_intfs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
> index 8836b31..3062167 100644
> --- a/drivers/staging/rtl8712/os_intfs.c
> +++ b/drivers/staging/rtl8712/os_intfs.c
> @@ -411,8 +411,7 @@ static int netdev_open(struct net_device *pnetdev)
>  			goto netdev_open_error;
>  		if (!padapter->dvobjpriv.inirp_init)
>  			goto netdev_open_error;
> -		else
> -			padapter->dvobjpriv.inirp_init(padapter);
> +		padapter->dvobjpriv.inirp_init(padapter);
>  		r8712_set_ps_mode(padapter, padapter->registrypriv.power_mgnt,
>  				  padapter->registrypriv.smart_ps);
>  	}
> --
> 2.7.4
>
> --
> 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/1488219268-3006-3-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Tue, Feb 28, 2017 at 2:49 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Mon, 27 Feb 2017, simran singhal wrote:
>
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
>>
>> This was done using Coccinelle:
>> @@
>> expression e2;
>> statement s1;
>> @@
>> if(e2) { ... return ...; }
>> -else
>>          s1
>
> One might be surprised that the following code was detected using the
> above semantic patch, because in the code below there is no return in the
> if branches.  Actually, as a special feature, when one has an if branch
> that ends in return, Coccinelle will skip through any gotos and see if the
> return is matched afterward.  Indeed it is a common pattern to have
>
> if (...) {
>    foo(x);
>    bar(y);
>    return -ENOMEM;
> }
>
> But the code can also be cut up as eg
>
> if (...) {
>    ret = -ENOMEM;
>    goto out;
> }
> ...
> out:
>    foo(x);
>    bar(y);
>    return ret;
>
> To avoid having to write multiple patterns for these cases, Coccinelle
> will just jump through the return in the second case, allowing the same
> pattern to match both of them.
>
Julia, Thanks for explaination. Its really helpful.

But I think there is no problem in removing else.
Because it will execute this

 padapter->dvobjpriv.inirp_init(padapter);

when if condition will not satisfy.

> julia
>
>>
>> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>> ---
>>  drivers/staging/rtl8712/os_intfs.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
>> index 8836b31..3062167 100644
>> --- a/drivers/staging/rtl8712/os_intfs.c
>> +++ b/drivers/staging/rtl8712/os_intfs.c
>> @@ -411,8 +411,7 @@ static int netdev_open(struct net_device *pnetdev)
>>                       goto netdev_open_error;
>>               if (!padapter->dvobjpriv.inirp_init)
>>                       goto netdev_open_error;
>> -             else
>> -                     padapter->dvobjpriv.inirp_init(padapter);
>> +             padapter->dvobjpriv.inirp_init(padapter);
>>               r8712_set_ps_mode(padapter, padapter->registrypriv.power_mgnt,
>>                                 padapter->registrypriv.smart_ps);
>>       }
>> --
>> 2.7.4
>>
>> --
>> 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/1488219268-3006-3-git-send-email-singhalsimran0%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>

Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return

[Julia Lawall]

On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 2:49 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Mon, 27 Feb 2017, simran singhal wrote:
> >
> >> This patch fixes the checkpatch warning that else is not generally
> >> useful after a break or return.
> >>
> >> This was done using Coccinelle:
> >> @@
> >> expression e2;
> >> statement s1;
> >> @@
> >> if(e2) { ... return ...; }
> >> -else
> >>          s1
> >
> > One might be surprised that the following code was detected using the
> > above semantic patch, because in the code below there is no return in the
> > if branches.  Actually, as a special feature, when one has an if branch
> > that ends in return, Coccinelle will skip through any gotos and see if the
> > return is matched afterward.  Indeed it is a common pattern to have
> >
> > if (...) {
> >    foo(x);
> >    bar(y);
> >    return -ENOMEM;
> > }
> >
> > But the code can also be cut up as eg
> >
> > if (...) {
> >    ret = -ENOMEM;
> >    goto out;
> > }
> > ...
> > out:
> >    foo(x);
> >    bar(y);
> >    return ret;
> >
> > To avoid having to write multiple patterns for these cases, Coccinelle
> > will just jump through the return in the second case, allowing the same
> > pattern to match both of them.
> >
> Julia, Thanks for explaination. Its really helpful.
>
> But I think there is no problem in removing else.
> Because it will execute this
>
>  padapter->dvobjpriv.inirp_init(padapter);
>
> when if condition will not satisfy.

It seems ok to me.

julia

>
> > julia
> >
> >>
> >> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> >> ---
> >>  drivers/staging/rtl8712/os_intfs.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
> >> index 8836b31..3062167 100644
> >> --- a/drivers/staging/rtl8712/os_intfs.c
> >> +++ b/drivers/staging/rtl8712/os_intfs.c
> >> @@ -411,8 +411,7 @@ static int netdev_open(struct net_device *pnetdev)
> >>                       goto netdev_open_error;
> >>               if (!padapter->dvobjpriv.inirp_init)
> >>                       goto netdev_open_error;
> >> -             else
> >> -                     padapter->dvobjpriv.inirp_init(padapter);
> >> +             padapter->dvobjpriv.inirp_init(padapter);
> >>               r8712_set_ps_mode(padapter, padapter->registrypriv.power_mgnt,
> >>                                 padapter->registrypriv.smart_ps);
> >>       }
> >> --
> >> 2.7.4
> >>
> >> --
> >> 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/1488219268-3006-3-git-send-email-singhalsimran0%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>

[PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

[simran singhal]

This patch fixes the checkpatch warning that else is not generally
useful after a break or return.

This was done using Coccinelle:
@@
expression e2;
statement s1;
@@
if(e2) { ... return ...; }
-else
         s1

Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
 drivers/staging/sm750fb/ddk750_sii164.c | 6 ++----
 drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 259006a..6906598 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
 	hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_HOT_PLUG_STATUS_MASK;
 	if (hotPlugValue = SII164_DETECT_HOT_PLUG_STATUS_ON)
 		return 1;
-	else
-		return 0;
+	return 0;
 }
 
 /*
@@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
 	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_MONITOR_STATE_MASK;
 	if (detectReg = SII164_DETECT_MONITOR_STATE_CHANGE)
 		return 1;
-	else
-		return 0;
+	return 0;
 }
 
 /*
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index a4ac07c..5997349 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
 	gpio_data = peek32(sw_i2c_data_gpio_data_reg);
 	if (gpio_data & (1 << sw_i2c_data_gpio))
 		return 1;
-	else
-		return 0;
+	return 0;
 }
 
 /*
@@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
 
 	if (i < 0xff)
 		return 0;
-	else
-		return -1;
+	return -1;
 }
 
 /*
-- 
2.7.4

Re: [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

[Joe Perches]

On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> This patch fixes the checkpatch warning that else is not generally
> useful after a break or return.
[]
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
[]
> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
>  
>  	if (i < 0xff)
>  		return 0;
> -	else
> -		return -1;
> +	return -1;

Assuming -1 is some sort of error,
it'd be a more common style to use

	if (i >= 0xff)
		return -1;

	return 0;

Looking at the code, it might make
sense to use something like:

	/* SDA still != 0 */
	if (i >= 0xff)
		return -1;

	return 0;
}

Re: [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Tue, Feb 28, 2017 at 1:01 AM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
> []
>> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> []
>> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
>>
>>       if (i < 0xff)
>>               return 0;
>> -     else
>> -             return -1;
>> +     return -1;
>
> Assuming -1 is some sort of error,
> it'd be a more common style to use
>
>         if (i >= 0xff)
>                 return -1;
>
>         return 0;
>
> Looking at the code, it might make
> sense to use something like:
>
>         /* SDA still != 0 */
>         if (i >= 0xff)
>                 return -1;
>
>         return 0;
> }
I will send v2.

Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

[Julia Lawall]

On Mon, 27 Feb 2017, simran singhal wrote:

> This patch fixes the checkpatch warning that else is not generally
> useful after a break or return.
>
> This was done using Coccinelle:
> @@
> expression e2;
> statement s1;
> @@
> if(e2) { ... return ...; }
> -else
>          s1
>
> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 6 ++----
>  drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++----
>  2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> index 259006a..6906598 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
>  	hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_HOT_PLUG_STATUS_MASK;
>  	if (hotPlugValue = SII164_DETECT_HOT_PLUG_STATUS_ON)
>  		return 1;
> -	else
> -		return 0;
> +	return 0;


Totally unrelated to what you are doing, but I wonder if these return
values should be true and false?  Perhaps it would help to see how the
return values are used at the calling context.

julia

>  }
>
>  /*
> @@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
>  	detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_MONITOR_STATE_MASK;
>  	if (detectReg = SII164_DETECT_MONITOR_STATE_CHANGE)
>  		return 1;
> -	else
> -		return 0;
> +	return 0;
>  }
>
>  /*
> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> index a4ac07c..5997349 100644
> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> @@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
>  	gpio_data = peek32(sw_i2c_data_gpio_data_reg);
>  	if (gpio_data & (1 << sw_i2c_data_gpio))
>  		return 1;
> -	else
> -		return 0;
> +	return 0;
>  }
>
>  /*
> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
>
>  	if (i < 0xff)
>  		return 0;
> -	else
> -		return -1;
> +	return -1;
>  }
>
>  /*
> --
> 2.7.4
>
> --
> 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/1488219268-3006-4-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Tue, Feb 28, 2017 at 2:45 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Mon, 27 Feb 2017, simran singhal wrote:
>
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
>>
>> This was done using Coccinelle:
>> @@
>> expression e2;
>> statement s1;
>> @@
>> if(e2) { ... return ...; }
>> -else
>>          s1
>>
>> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>> ---
>>  drivers/staging/sm750fb/ddk750_sii164.c | 6 ++----
>>  drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++----
>>  2 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
>> index 259006a..6906598 100644
>> --- a/drivers/staging/sm750fb/ddk750_sii164.c
>> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
>> @@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
>>       hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_HOT_PLUG_STATUS_MASK;
>>       if (hotPlugValue = SII164_DETECT_HOT_PLUG_STATUS_ON)
>>               return 1;
>> -     else
>> -             return 0;
>> +     return 0;
>
>
> Totally unrelated to what you are doing, but I wonder if these return
> values should be true and false?  Perhaps it would help to see how the
> return values are used at the calling context.
>
you want me to go ahead with what Joe mentioned and also do the same
changes here also.
> julia
>
>>  }
>>
>>  /*
>> @@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
>>       detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_MONITOR_STATE_MASK;
>>       if (detectReg = SII164_DETECT_MONITOR_STATE_CHANGE)
>>               return 1;
>> -     else
>> -             return 0;
>> +     return 0;
>>  }
>>
>>  /*
>> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
>> index a4ac07c..5997349 100644
>> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
>> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
>> @@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
>>       gpio_data = peek32(sw_i2c_data_gpio_data_reg);
>>       if (gpio_data & (1 << sw_i2c_data_gpio))
>>               return 1;
>> -     else
>> -             return 0;
>> +     return 0;
>>  }
>>
>>  /*
>> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
>>
>>       if (i < 0xff)
>>               return 0;
>> -     else
>> -             return -1;
>> +     return -1;
>>  }
>>
>>  /*
>> --
>> 2.7.4
>>
>> --
>> 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/1488219268-3006-4-git-send-email-singhalsimran0%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>

Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

[Julia Lawall]

On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 2:45 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Mon, 27 Feb 2017, simran singhal wrote:
> >
> >> This patch fixes the checkpatch warning that else is not generally
> >> useful after a break or return.
> >>
> >> This was done using Coccinelle:
> >> @@
> >> expression e2;
> >> statement s1;
> >> @@
> >> if(e2) { ... return ...; }
> >> -else
> >>          s1
> >>
> >> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> >> ---
> >>  drivers/staging/sm750fb/ddk750_sii164.c | 6 ++----
> >>  drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++----
> >>  2 files changed, 4 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> >> index 259006a..6906598 100644
> >> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> >> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> >> @@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
> >>       hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_HOT_PLUG_STATUS_MASK;
> >>       if (hotPlugValue = SII164_DETECT_HOT_PLUG_STATUS_ON)
> >>               return 1;
> >> -     else
> >> -             return 0;
> >> +     return 0;
> >
> >
> > Totally unrelated to what you are doing, but I wonder if these return
> > values should be true and false?  Perhaps it would help to see how the
> > return values are used at the calling context.
> >
> you want me to go ahead with what Joe mentioned and also do the same
> changes here also.

I think that it would be best to complete what you have underway already.
When those patches are picked up by Greg, you can look into the
possibility of using true and false.

julia

> > julia
> >
> >>  }
> >>
> >>  /*
> >> @@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
> >>       detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_MONITOR_STATE_MASK;
> >>       if (detectReg = SII164_DETECT_MONITOR_STATE_CHANGE)
> >>               return 1;
> >> -     else
> >> -             return 0;
> >> +     return 0;
> >>  }
> >>
> >>  /*
> >> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
> >> index a4ac07c..5997349 100644
> >> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> >> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> >> @@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
> >>       gpio_data = peek32(sw_i2c_data_gpio_data_reg);
> >>       if (gpio_data & (1 << sw_i2c_data_gpio))
> >>               return 1;
> >> -     else
> >> -             return 0;
> >> +     return 0;
> >>  }
> >>
> >>  /*
> >> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
> >>
> >>       if (i < 0xff)
> >>               return 0;
> >> -     else
> >> -             return -1;
> >> +     return -1;
> >>  }
> >>
> >>  /*
> >> --
> >> 2.7.4
> >>
> >> --
> >> 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/1488219268-3006-4-git-send-email-singhalsimran0%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
> --
> 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/CALrZqyNXbn1SZ8Daws7yKkYm-C-U9FHyJ_eRxvuem1Fk%2BMqs%3Dg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Wed, Mar 1, 2017 at 12:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:
>
>> On Tue, Feb 28, 2017 at 2:45 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >
>> >
>> > On Mon, 27 Feb 2017, simran singhal wrote:
>> >
>> >> This patch fixes the checkpatch warning that else is not generally
>> >> useful after a break or return.
>> >>
>> >> This was done using Coccinelle:
>> >> @@
>> >> expression e2;
>> >> statement s1;
>> >> @@
>> >> if(e2) { ... return ...; }
>> >> -else
>> >>          s1
>> >>
>> >> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>> >> ---
>> >>  drivers/staging/sm750fb/ddk750_sii164.c | 6 ++----
>> >>  drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++----
>> >>  2 files changed, 4 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
>> >> index 259006a..6906598 100644
>> >> --- a/drivers/staging/sm750fb/ddk750_sii164.c
>> >> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
>> >> @@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
>> >>       hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_HOT_PLUG_STATUS_MASK;
>> >>       if (hotPlugValue = SII164_DETECT_HOT_PLUG_STATUS_ON)
>> >>               return 1;
>> >> -     else
>> >> -             return 0;
>> >> +     return 0;
>> >
>> >
>> > Totally unrelated to what you are doing, but I wonder if these return
>> > values should be true and false?  Perhaps it would help to see how the
>> > return values are used at the calling context.
>> >
>> you want me to go ahead with what Joe mentioned and also do the same
>> changes here also.
>
> I think that it would be best to complete what you have underway already.
> When those patches are picked up by Greg, you can look into the
> possibility of using true and false.
>
Thanks, so for now I can ignore this.
> julia
>
>> > julia
>> >
>> >>  }
>> >>
>> >>  /*
>> >> @@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
>> >>       detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & SII164_DETECT_MONITOR_STATE_MASK;
>> >>       if (detectReg = SII164_DETECT_MONITOR_STATE_CHANGE)
>> >>               return 1;
>> >> -     else
>> >> -             return 0;
>> >> +     return 0;
>> >>  }
>> >>
>> >>  /*
>> >> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
>> >> index a4ac07c..5997349 100644
>> >> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
>> >> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
>> >> @@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
>> >>       gpio_data = peek32(sw_i2c_data_gpio_data_reg);
>> >>       if (gpio_data & (1 << sw_i2c_data_gpio))
>> >>               return 1;
>> >> -     else
>> >> -             return 0;
>> >> +     return 0;
>> >>  }
>> >>
>> >>  /*
>> >> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
>> >>
>> >>       if (i < 0xff)
>> >>               return 0;
>> >> -     else
>> >> -             return -1;
>> >> +     return -1;
>> >>  }
>> >>
>> >>  /*
>> >> --
>> >> 2.7.4
>> >>
>> >> --
>> >> 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/1488219268-3006-4-git-send-email-singhalsimran0%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>
>> --
>> 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/CALrZqyNXbn1SZ8Daws7yKkYm-C-U9FHyJ_eRxvuem1Fk%2BMqs%3Dg%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>

[PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

[simran singhal]

This patch fixes the checkpatch warning that else is not generally
useful after a break or return.

This was done using Coccinelle:
@@
expression e2;
statement s1;
@@
if(e2) { ... return ...; }
-else
         s1

Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
 drivers/staging/gdm724x/gdm_endian.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
index d0b43e2..4ef728f 100644
--- a/drivers/staging/gdm724x/gdm_endian.c
+++ b/drivers/staging/gdm724x/gdm_endian.c
@@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
 {
 	if (ed->dev_ed = ENDIANNESS_LITTLE)
 		return (__force __dev16)cpu_to_le16(x);
-	else
-		return (__force __dev16)cpu_to_be16(x);
+	return (__force __dev16)cpu_to_be16(x);
 }
 
 u16 gdm_dev16_to_cpu(struct gdm_endian *ed, __dev16 x)
 {
 	if (ed->dev_ed = ENDIANNESS_LITTLE)
 		return le16_to_cpu((__force __le16)x);
-	else
-		return be16_to_cpu((__force __be16)x);
+	return be16_to_cpu((__force __be16)x);
 }
 
 __dev32 gdm_cpu_to_dev32(struct gdm_endian *ed, u32 x)
 {
 	if (ed->dev_ed = ENDIANNESS_LITTLE)
 		return (__force __dev32)cpu_to_le32(x);
-	else
-		return (__force __dev32)cpu_to_be32(x);
+	return (__force __dev32)cpu_to_be32(x);
 }
 
 u32 gdm_dev32_to_cpu(struct gdm_endian *ed, __dev32 x)
 {
 	if (ed->dev_ed = ENDIANNESS_LITTLE)
 		return le32_to_cpu((__force __le32)x);
-	else
-		return be32_to_cpu((__force __be32)x);
+	return be32_to_cpu((__force __be32)x);
 }
-- 
2.7.4

Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

[Joe Perches]

On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> This patch fixes the checkpatch warning that else is not generally
> useful after a break or return.

> This was done using Coccinelle:
> @@
> expression e2;
> statement s1;
> @@
> if(e2) { ... return ...; }
> -else
>          s1
[]
> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
[]
> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
>  {
>  	if (ed->dev_ed = ENDIANNESS_LITTLE)
>  		return (__force __dev16)cpu_to_le16(x);
> -	else
> -		return (__force __dev16)cpu_to_be16(x);
> +	return (__force __dev16)cpu_to_be16(x);

again, not a checkpatch message for any of the
suggested modified hunks.

Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
>
>> This was done using Coccinelle:
>> @@
>> expression e2;
>> statement s1;
>> @@
>> if(e2) { ... return ...; }
>> -else
>>          s1
> []
>> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> []
>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
>>  {
>>       if (ed->dev_ed = ENDIANNESS_LITTLE)
>>               return (__force __dev16)cpu_to_le16(x);
>> -     else
>> -             return (__force __dev16)cpu_to_be16(x);
>> +     return (__force __dev16)cpu_to_be16(x);
>
> again, not a checkpatch message for any of the
> suggested modified hunks.
>
So, I have to change commit message.

Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
<singhalsimran0@gmail.com> wrote:
> On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
>> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>>> This patch fixes the checkpatch warning that else is not generally
>>> useful after a break or return.
>>
>>> This was done using Coccinelle:
>>> @@
>>> expression e2;
>>> statement s1;
>>> @@
>>> if(e2) { ... return ...; }
>>> -else
>>>          s1
>> []
>>> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
>> []
>>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
>>>  {
>>>       if (ed->dev_ed = ENDIANNESS_LITTLE)
>>>               return (__force __dev16)cpu_to_le16(x);
>>> -     else
>>> -             return (__force __dev16)cpu_to_be16(x);
>>> +     return (__force __dev16)cpu_to_be16(x);
>>
>> again, not a checkpatch message for any of the
>> suggested modified hunks.
>>
I am not getting what's the problem in removing else or may be I
am wrong you just want to say that I should change the commit message.

Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

[Julia Lawall]

On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
> <singhalsimran0@gmail.com> wrote:
> > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> >> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> >>> This patch fixes the checkpatch warning that else is not generally
> >>> useful after a break or return.
> >>
> >>> This was done using Coccinelle:
> >>> @@
> >>> expression e2;
> >>> statement s1;
> >>> @@
> >>> if(e2) { ... return ...; }
> >>> -else
> >>>          s1
> >> []
> >>> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> >> []
> >>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> >>>  {
> >>>       if (ed->dev_ed = ENDIANNESS_LITTLE)
> >>>               return (__force __dev16)cpu_to_le16(x);
> >>> -     else
> >>> -             return (__force __dev16)cpu_to_be16(x);
> >>> +     return (__force __dev16)cpu_to_be16(x);
> >>
> >> again, not a checkpatch message for any of the
> >> suggested modified hunks.
> >>
> I am not getting what's the problem in removing else or may be I
> am wrong you just want to say that I should change the commit message.

Yes, I think that the issue is just the commit message.  Was it really
checkpatch that motivated you to do this?  Joe maintains checkpatch, and
he doesn't think that it gives such a warning.

julia

Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

[Joe Perches]

On Wed, 2017-03-01 at 00:41 +0530, SIMRAN SINGHAL wrote:
> On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
> <singhalsimran0@gmail.com> wrote:
> > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> > > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> > > > This patch fixes the checkpatch warning that else is not generally
> > > > useful after a break or return.
> > > > This was done using Coccinelle:
> > > > @@
> > > > expression e2;
> > > > statement s1;
> > > > @@
> > > > if(e2) { ... return ...; }
> > > > -else
> > > >          s1
> > > 
> > > []
> > > > diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> > > 
> > > []
> > > > @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> > > >  {
> > > >       if (ed->dev_ed = ENDIANNESS_LITTLE)
> > > >               return (__force __dev16)cpu_to_le16(x);
> > > > -     else
> > > > -             return (__force __dev16)cpu_to_be16(x);
> > > > +     return (__force __dev16)cpu_to_be16(x);
> > > 
> > > again, not a checkpatch message for any of the
> > > suggested modified hunks.
> > > 
> 
> I am not getting what's the problem in removing else or may be I
> am wrong you just want to say that I should change the commit message.

2 things:

1: The commit message is incorrect.
2: This form is fundamentally OK:

	if (foo)
		return bar;
	else
		return baz;


So I think this patch is not good.

Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

[Julia Lawall]

On Tue, 28 Feb 2017, Joe Perches wrote:

> On Wed, 2017-03-01 at 00:41 +0530, SIMRAN SINGHAL wrote:
> > On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
> > <singhalsimran0@gmail.com> wrote:
> > > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> > > > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> > > > > This patch fixes the checkpatch warning that else is not generally
> > > > > useful after a break or return.
> > > > > This was done using Coccinelle:
> > > > > @@
> > > > > expression e2;
> > > > > statement s1;
> > > > > @@
> > > > > if(e2) { ... return ...; }
> > > > > -else
> > > > >          s1
> > > >
> > > > []
> > > > > diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> > > >
> > > > []
> > > > > @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> > > > >  {
> > > > >       if (ed->dev_ed = ENDIANNESS_LITTLE)
> > > > >               return (__force __dev16)cpu_to_le16(x);
> > > > > -     else
> > > > > -             return (__force __dev16)cpu_to_be16(x);
> > > > > +     return (__force __dev16)cpu_to_be16(x);
> > > >
> > > > again, not a checkpatch message for any of the
> > > > suggested modified hunks.
> > > >
> >
> > I am not getting what's the problem in removing else or may be I
> > am wrong you just want to say that I should change the commit message.
>
> 2 things:
>
> 1: The commit message is incorrect.
> 2: This form is fundamentally OK:
>
> 	if (foo)
> 		return bar;
> 	else
> 		return baz;
>
>
> So I think this patch is not good.

I agree in this case.  The two branches are quite parallel.  In some of
the other patches, the if was looking for the absence of some resource or
the failure of something, so there was a clear distinction between one
branch being cleanup on failure and the other branch being the continuing
successful computation, even if it is just to return a success indicator.

julia

Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

[Joe Perches]

On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> This patch fixes the checkpatch warning that else is not generally
> useful after a break or return.

checkpatch doesn't actually warn for this style

	if (foo)
		return bar;
	else
		return baz;

> @@
> expression e2;
> statement s1;
> @@
> if(e2) { ... return ...; }
> -else
>          s1
> 
> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> ---
>  drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 3 +--
>  drivers/staging/lustre/lustre/ldlm/ldlm_pool.c      | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> index fbbd8a5..02d49b7 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct lnet_process_id id, __u32 ipaddr)
>  
>  	if (!count)
>  		return -ENOENT;
> -	else
> -		return 0;
> +	return 0;
>  }
>  
>  void
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> index cf3fc57..ac32c82 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> @@ -338,8 +338,7 @@ static int ldlm_cli_pool_shrink(struct ldlm_pool *pl,
>  
>  	if (nr = 0)
>  		return (unused / 100) * sysctl_vfs_cache_pressure;
> -	else
> -		return ldlm_cancel_lru(ns, nr, LCF_ASYNC, LDLM_LRU_FLAG_SHRINK);
> +	return ldlm_cancel_lru(ns, nr, LCF_ASYNC, LDLM_LRU_FLAG_SHRINK);
>  }
>  
>  static const struct ldlm_pool_ops ldlm_cli_pool_ops = {

Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Tue, Feb 28, 2017 at 12:55 AM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
>
> checkpatch doesn't actually warn for this style
>
>         if (foo)
>                 return bar;
>         else
>                 return baz;
>
ok, My bad
so, I have to change commit message as checkpatch doesn't warn for this style.
>> @@
>> expression e2;
>> statement s1;
>> @@
>> if(e2) { ... return ...; }
>> -else
>>          s1
>>
>> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>> ---
>>  drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 3 +--
>>  drivers/staging/lustre/lustre/ldlm/ldlm_pool.c      | 3 +--
>>  2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
>> index fbbd8a5..02d49b7 100644
>> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
>> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
>> @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct lnet_process_id id, __u32 ipaddr)
>>
>>       if (!count)
>>               return -ENOENT;
>> -     else
>> -             return 0;
>> +     return 0;
>>  }
>>
>>  void
>> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
>> index cf3fc57..ac32c82 100644
>> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
>> @@ -338,8 +338,7 @@ static int ldlm_cli_pool_shrink(struct ldlm_pool *pl,
>>
>>       if (nr = 0)
>>               return (unused / 100) * sysctl_vfs_cache_pressure;
>> -     else
>> -             return ldlm_cancel_lru(ns, nr, LCF_ASYNC, LDLM_LRU_FLAG_SHRINK);
>> +     return ldlm_cancel_lru(ns, nr, LCF_ASYNC, LDLM_LRU_FLAG_SHRINK);
>>  }
>>
>>  static const struct ldlm_pool_ops ldlm_cli_pool_ops = {

Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

[Joe Perches]

On Tue, 2017-02-28 at 01:51 +0530, SIMRAN SINGHAL wrote:
> On Tue, Feb 28, 2017 at 12:55 AM, Joe Perches <joe@perches.com> wrote:
> > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> > > This patch fixes the checkpatch warning that else is not generally
> > > useful after a break or return.
> > 
> > checkpatch doesn't actually warn for this style
> > 
> >         if (foo)
> >                 return bar;
> >         else
> >                 return baz;
> > 
> 
> ok, My bad
> so, I have to change commit message as checkpatch doesn't warn for this style.

Perhaps better would be to leave them unchanged instead.

> > > diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
[]
> > > @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct lnet_process_id id, __u32 ipaddr)
> > > 
> > >       if (!count)
> > >               return -ENOENT;
> > > -     else
> > > -             return 0;
> > > +     return 0;

There might be a case for this one.
error returns are generally in the form

{
	[...]

	err = func(...);
	if (err < 0)
		return err;

	return 0;
}

Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

[SIMRAN SINGHAL]

On Tue, Feb 28, 2017 at 2:13 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2017-02-28 at 01:51 +0530, SIMRAN SINGHAL wrote:
>> On Tue, Feb 28, 2017 at 12:55 AM, Joe Perches <joe@perches.com> wrote:
>> > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>> > > This patch fixes the checkpatch warning that else is not generally
>> > > useful after a break or return.
>> >
>> > checkpatch doesn't actually warn for this style
>> >
>> >         if (foo)
>> >                 return bar;
>> >         else
>> >                 return baz;
>> >
>>
>> ok, My bad
>> so, I have to change commit message as checkpatch doesn't warn for this style.
>
> Perhaps better would be to leave them unchanged instead.
>
>> > > diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> []
>> > > @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct lnet_process_id id, __u32 ipaddr)
>> > >
>> > >       if (!count)
>> > >               return -ENOENT;
>> > > -     else
>> > > -             return 0;
>> > > +     return 0;
>
> There might be a case for this one.
> error returns are generally in the form
>
> {
>         [...]
>
>         err = func(...);
>         if (err < 0)
>                 return err;
>
>         return 0;
> }
Not sure, what's the problem in removing else as according to me
there is no use of else.

In this case if (if condition) does not satisfy then else condition will
be satisfied and function will return 0.

Re: [Outreachy kernel] Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

[Julia Lawall]

On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 2:13 AM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2017-02-28 at 01:51 +0530, SIMRAN SINGHAL wrote:
> >> On Tue, Feb 28, 2017 at 12:55 AM, Joe Perches <joe@perches.com> wrote:
> >> > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> >> > > This patch fixes the checkpatch warning that else is not generally
> >> > > useful after a break or return.
> >> >
> >> > checkpatch doesn't actually warn for this style
> >> >
> >> >         if (foo)
> >> >                 return bar;
> >> >         else
> >> >                 return baz;
> >> >
> >>
> >> ok, My bad
> >> so, I have to change commit message as checkpatch doesn't warn for this style.
> >
> > Perhaps better would be to leave them unchanged instead.
> >
> >> > > diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> > []
> >> > > @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct lnet_process_id id, __u32 ipaddr)
> >> > >
> >> > >       if (!count)
> >> > >               return -ENOENT;
> >> > > -     else
> >> > > -             return 0;
> >> > > +     return 0;
> >
> > There might be a case for this one.
> > error returns are generally in the form
> >
> > {
> >         [...]
> >
> >         err = func(...);
> >         if (err < 0)
> >                 return err;
> >
> >         return 0;
> > }
> Not sure, what's the problem in removing else as according to me
> there is no use of else.
>
> In this case if (if condition) does not satisfy then else condition will
> be satisfied and function will return 0.

I think that "there might be a case for" was a positive comment.  In any
case, it looks nicer to me if success is outside of a conditional and
failure is under a conditional.  Or to be more precise, Coccinelle bug
finding rules tend to work better if that property is respected.

julia

>
> --
> 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/CALrZqyP-BRPj4jEFgU%3DB_AV7E4-tJfgaXwwxRhUprLg_4gWQkg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>