All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-27 18:26 ` simran singhal
  0 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:14 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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



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

* [PATCH 2/5] staging: rtl8192u: Remove unnecessary else after return
  2017-02-27 18:26 ` simran singhal
@ 2017-02-27 18:26   ` simran singhal
  -1 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:14 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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



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

* [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
  2017-02-27 18:26 ` simran singhal
@ 2017-02-27 18:26   ` simran singhal
  -1 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:14 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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



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

* [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
  2017-02-27 18:26 ` simran singhal
@ 2017-02-27 18:26   ` simran singhal
  -1 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:14 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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



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

* [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
  2017-02-27 18:26 ` simran singhal
@ 2017-02-27 18:26   ` simran singhal
  -1 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:14 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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



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

* [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-27 18:26 ` simran singhal
  0 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:26 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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


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

* [PATCH 2/5] staging: rtl8192u: Remove unnecessary else after return
@ 2017-02-27 18:26   ` simran singhal
  0 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:26 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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


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

* [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
@ 2017-02-27 18:26   ` simran singhal
  0 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:26 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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


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

* [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-27 18:26   ` simran singhal
  0 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:26 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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


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

* [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-27 18:26   ` simran singhal
  0 siblings, 0 replies; 65+ messages in thread
From: simran singhal @ 2017-02-27 18:26 UTC (permalink / raw)
  To: gregkh; +Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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


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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
  2017-02-27 18:26 ` simran singhal
  (?)
@ 2017-02-27 19:25   ` Joe Perches
  -1 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:25 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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 = {


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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-27 19:25   ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:25 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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 = {

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

* [lustre-devel] [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-27 19:25   ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:25 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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 = {

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

* Re: [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
  2017-02-27 18:26   ` simran singhal
  (?)
@ 2017-02-27 19:31     ` Joe Perches
  -1 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:31 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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;
}


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

* Re: [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-27 19:31     ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:31 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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;
}

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

* [lustre-devel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-27 19:31     ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:31 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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;
}

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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
  2017-02-27 18:26   ` simran singhal
  (?)
@ 2017-02-27 19:41     ` Joe Perches
  -1 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:41 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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.



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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-27 19:41     ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:41 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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.


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

* [lustre-devel] [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-27 19:41     ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 19:41 UTC (permalink / raw)
  To: simran singhal, gregkh
  Cc: lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel

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.

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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
  2017-02-27 19:41     ` Joe Perches
@ 2017-02-27 20:31       ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-27 20:19 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.


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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
  2017-02-27 19:25   ` Joe Perches
@ 2017-02-27 20:33     ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-27 20:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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 = {


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

* Re: [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
  2017-02-27 19:31     ` Joe Perches
@ 2017-02-27 20:34       ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-27 20:22 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.


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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-27 20:31       ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-27 20:31 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.

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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-27 20:33     ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-27 20:33 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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 = {

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

* Re: [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-27 20:34       ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-27 20:34 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.

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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
  2017-02-27 20:33     ` SIMRAN SINGHAL
  (?)
@ 2017-02-27 20:43       ` Joe Perches
  -1 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 20:43 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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;
}


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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-27 20:43       ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 20:43 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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;
}

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

* [lustre-devel] [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-27 20:43       ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-27 20:43 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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;
}

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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
  2017-02-27 18:26   ` simran singhal
  (?)
@ 2017-02-27 21:15     ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:15 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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.
>


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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-27 21:15     ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:15 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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.
>

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

* [lustre-devel] [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-27 21:15     ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:15 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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 at googlegroups.com.
> To post to this group, send email to outreachy-kernel at 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.
>

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

* Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
  2017-02-27 18:26   ` simran singhal
  (?)
@ 2017-02-27 21:19     ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:19 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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.
>


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

* Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
@ 2017-02-27 21:19     ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:19 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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.
>

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

* [lustre-devel] [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
@ 2017-02-27 21:19     ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:19 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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 at googlegroups.com.
> To post to this group, send email to outreachy-kernel at 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.
>

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

* Re: [Outreachy kernel] [PATCH 2/5] staging: rtl8192u: Remove unnecessary else after return
  2017-02-27 18:26   ` simran singhal
  (?)
@ 2017-02-27 21:25     ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:25 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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


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

* Re: [Outreachy kernel] [PATCH 2/5] staging: rtl8192u: Remove unnecessary else after return
@ 2017-02-27 21:25     ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:25 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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

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

* [lustre-devel] [Outreachy kernel] [PATCH 2/5] staging: rtl8192u: Remove unnecessary else after return
@ 2017-02-27 21:25     ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-27 21:25 UTC (permalink / raw)
  To: simran singhal
  Cc: gregkh, lustre-devel, devel, linux-kernel, linux-fbdev, outreachy-kernel



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

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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
  2017-02-27 21:15     ` Julia Lawall
@ 2017-02-28 19:08       ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 18:56 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.
>>


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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
  2017-02-28 19:08       ` SIMRAN SINGHAL
  (?)
@ 2017-02-28 19:00         ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:00 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel



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.
>


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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-28 19:00         ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:00 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel



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.
>

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

* [lustre-devel] [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-28 19:00         ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:00 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel



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 at googlegroups.com.
> >> To post to this group, send email to outreachy-kernel at 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 at googlegroups.com.
> To post to this group, send email to outreachy-kernel at 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.
>

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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
  2017-02-27 20:43       ` Joe Perches
@ 2017-02-28 19:13         ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:01 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.


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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
  2017-02-28 19:00         ` Julia Lawall
@ 2017-02-28 19:17           ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:05 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.
>>


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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-28 19:08       ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:08 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.
>>

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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
  2017-02-27 20:31       ` SIMRAN SINGHAL
@ 2017-02-28 19:23         ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:11 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.


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

* Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-28 19:13         ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:13 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.

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

* Re: [Outreachy kernel] Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
  2017-02-28 19:13         ` SIMRAN SINGHAL
  (?)
@ 2017-02-28 19:14           ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:14 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Joe Perches, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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.
>


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

* Re: [Outreachy kernel] Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-28 19:14           ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:14 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Joe Perches, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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.
>

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

* [lustre-devel] [Outreachy kernel] Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return
@ 2017-02-28 19:14           ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:14 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Joe Perches, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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 at googlegroups.com.
> To post to this group, send email to outreachy-kernel at 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.
>

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

* Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
  2017-02-27 21:19     ` Julia Lawall
@ 2017-02-28 19:29       ` SIMRAN SINGHAL
  -1 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:17 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.
>>


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

* Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return
@ 2017-02-28 19:17           ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:17 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.
>>

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

* Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
  2017-02-28 19:23         ` SIMRAN SINGHAL
  (?)
@ 2017-02-28 19:18           ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:18 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Joe Perches, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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


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

* Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-28 19:18           ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:18 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Joe Perches, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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

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

* [lustre-devel] [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-28 19:18           ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:18 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Joe Perches, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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

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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
  2017-02-28 19:23         ` SIMRAN SINGHAL
  (?)
@ 2017-02-28 19:18           ` Joe Perches
  -1 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-28 19:18 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.


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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-28 19:18           ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-28 19:18 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.

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

* [lustre-devel] [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-28 19:18           ` Joe Perches
  0 siblings, 0 replies; 65+ messages in thread
From: Joe Perches @ 2017-02-28 19:18 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.

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

* Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
  2017-02-28 19:29       ` SIMRAN SINGHAL
  (?)
@ 2017-02-28 19:19         ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:19 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel



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.
> >>
>


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

* Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
@ 2017-02-28 19:19         ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:19 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel



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.
> >>
>

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

* [lustre-devel] [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
@ 2017-02-28 19:19         ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:19 UTC (permalink / raw)
  To: SIMRAN SINGHAL
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel



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 at googlegroups.com.
> >> To post to this group, send email to outreachy-kernel at 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.
> >>
>

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

* Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
  2017-02-28 19:18           ` Joe Perches
  (?)
@ 2017-02-28 19:21             ` Julia Lawall
  -1 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: SIMRAN SINGHAL, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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


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

* Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-28 19:21             ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: SIMRAN SINGHAL, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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

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

* [lustre-devel] [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-28 19:21             ` Julia Lawall
  0 siblings, 0 replies; 65+ messages in thread
From: Julia Lawall @ 2017-02-28 19:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: SIMRAN SINGHAL, Greg KH, lustre-devel, devel, linux-kernel,
	linux-fbdev, outreachy-kernel



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

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

* Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return
@ 2017-02-28 19:23         ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:23 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.

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

* Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return
@ 2017-02-28 19:29       ` SIMRAN SINGHAL
  0 siblings, 0 replies; 65+ messages in thread
From: SIMRAN SINGHAL @ 2017-02-28 19:29 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Greg KH, lustre-devel, devel, linux-kernel, linux-fbdev,
	outreachy-kernel

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.
>>

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

end of thread, other threads:[~2017-02-28 19:29 UTC | newest]

Thread overview: 65+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-27 18:14 [PATCH 1/5] staging: lustre: Remove unnecessary else after return simran singhal
2017-02-27 18:26 ` simran singhal
2017-02-27 18:14 ` [PATCH 2/5] staging: rtl8192u: " simran singhal
2017-02-27 18:26   ` simran singhal
2017-02-27 21:25   ` [Outreachy kernel] " Julia Lawall
2017-02-27 21:25     ` [lustre-devel] " Julia Lawall
2017-02-27 21:25     ` Julia Lawall
2017-02-27 18:14 ` [PATCH 3/5] staging: rtl8712: " simran singhal
2017-02-27 18:26   ` simran singhal
2017-02-27 21:19   ` [Outreachy kernel] " Julia Lawall
2017-02-27 21:19     ` [lustre-devel] " Julia Lawall
2017-02-27 21:19     ` Julia Lawall
2017-02-28 19:17     ` SIMRAN SINGHAL
2017-02-28 19:29       ` SIMRAN SINGHAL
2017-02-28 19:19       ` Julia Lawall
2017-02-28 19:19         ` [lustre-devel] " Julia Lawall
2017-02-28 19:19         ` Julia Lawall
2017-02-27 18:14 ` [PATCH 4/5] staging: sm750fb: " simran singhal
2017-02-27 18:26   ` simran singhal
2017-02-27 19:31   ` Joe Perches
2017-02-27 19:31     ` [lustre-devel] " Joe Perches
2017-02-27 19:31     ` Joe Perches
2017-02-27 20:22     ` SIMRAN SINGHAL
2017-02-27 20:34       ` SIMRAN SINGHAL
2017-02-27 21:15   ` [Outreachy kernel] " Julia Lawall
2017-02-27 21:15     ` [lustre-devel] " Julia Lawall
2017-02-27 21:15     ` Julia Lawall
2017-02-28 18:56     ` SIMRAN SINGHAL
2017-02-28 19:08       ` SIMRAN SINGHAL
2017-02-28 19:00       ` Julia Lawall
2017-02-28 19:00         ` [lustre-devel] " Julia Lawall
2017-02-28 19:00         ` Julia Lawall
2017-02-28 19:05         ` SIMRAN SINGHAL
2017-02-28 19:17           ` SIMRAN SINGHAL
2017-02-27 18:14 ` [PATCH 5/5] staging: gdm724x: " simran singhal
2017-02-27 18:26   ` simran singhal
2017-02-27 19:41   ` Joe Perches
2017-02-27 19:41     ` [lustre-devel] " Joe Perches
2017-02-27 19:41     ` Joe Perches
2017-02-27 20:19     ` SIMRAN SINGHAL
2017-02-27 20:31       ` SIMRAN SINGHAL
2017-02-28 19:11       ` SIMRAN SINGHAL
2017-02-28 19:23         ` SIMRAN SINGHAL
2017-02-28 19:18         ` [Outreachy kernel] " Julia Lawall
2017-02-28 19:18           ` [lustre-devel] " Julia Lawall
2017-02-28 19:18           ` Julia Lawall
2017-02-28 19:18         ` Joe Perches
2017-02-28 19:18           ` [lustre-devel] " Joe Perches
2017-02-28 19:18           ` Joe Perches
2017-02-28 19:21           ` [Outreachy kernel] " Julia Lawall
2017-02-28 19:21             ` [lustre-devel] " Julia Lawall
2017-02-28 19:21             ` Julia Lawall
2017-02-27 19:25 ` [PATCH 1/5] staging: lustre: " Joe Perches
2017-02-27 19:25   ` [lustre-devel] " Joe Perches
2017-02-27 19:25   ` Joe Perches
2017-02-27 20:21   ` SIMRAN SINGHAL
2017-02-27 20:33     ` SIMRAN SINGHAL
2017-02-27 20:43     ` Joe Perches
2017-02-27 20:43       ` [lustre-devel] " Joe Perches
2017-02-27 20:43       ` Joe Perches
2017-02-28 19:01       ` SIMRAN SINGHAL
2017-02-28 19:13         ` SIMRAN SINGHAL
2017-02-28 19:14         ` [Outreachy kernel] " Julia Lawall
2017-02-28 19:14           ` [lustre-devel] " Julia Lawall
2017-02-28 19:14           ` Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.