linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/13] make return of 0 explicit
@ 2014-05-19  4:31 Julia Lawall
  2014-05-19  4:31 ` [PATCH 1/13] libertas: " Julia Lawall
                   ` (11 more replies)
  0 siblings, 12 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: brcm80211-dev-list
  Cc: kernel-janitors, alsa-devel, coreteam, netfilter,
	netfilter-devel, linux-scsi, linux-kernel, libertas-dev,
	linux-wireless, netdev, linux-usb, devel, linux-raid,
	oprofile-list, linux-s390

Sometimes a local variable is used as a return value in a case where the
return value is always 0.  The result is more apparent if this variable is
just replaced by 0.  This is done by the following semantic patch, although
some cleanups may be needed. (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
identifier reti;
expression e;
position p;
@@

ret@reti = 0;
... when != \(ret = e\|ret &= e\|ret |= e\|ret += e\|ret -= e\|ret *= e\|ret ^= e\)
    when != \(++ret\|--ret\|ret++\|ret--\)
    when != &ret
return ret;@p

@bad1 exists@
expression e != 0;
local idexpression r.ret;
position r.p;
@@

 \(ret = e\|ret &= e\|ret |= e\|ret += e\|ret -= e\|ret *= e\|ret ^= e\|++ret\|--ret\|ret++\|ret--\|&ret\)
 ...
 return ret;@p

@bad2 exists@
identifier r.reti;
position r.p;
identifier f;
type T;
@@

f(...,T reti,...) {
 ...
 return reti;@p
}

@change depends on !bad1 && !bad2@
position r.p;
local idexpression r.ret;
identifier f;
@@

f(...) { <+...
return 
-ret
+0
;@p
...+> }

@rewrite@
local idexpression r.ret;
expression e;
position p;
identifier change.f;
@@

f(...) { <+...
 \(ret@p = e\|&ret@p\)
...+> }

@depends on change@
local idexpression r.ret;
position p != rewrite.p;
identifier change.f;
@@

f(...) { <+...
(
break;
|
ret = 0;
... when exists
ret@p
... when any
|
- ret = 0;
)
...+> }

@depends on change@
identifier r.reti;
type T;
constant C;
identifier change.f;
@@

f(...) { ... when any
-T reti = C;
 ... when != reti
     when strict
 }

@depends on change@
identifier r.reti;
type T;
identifier change.f;
@@

f(...) { ... when any
-T reti;
 ... when != reti
     when strict
 }
// </smpl>

The first four rules detect cases where only 0 reaches a return.  The
remaining rules clean up any variable initializations or declarations that
are made unnecessary by this change.


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

* [PATCH 1/13] libertas: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  7:43   ` walter harms
  2014-05-19 12:45   ` Sergei Shtylyov
  2014-05-19  4:31 ` [PATCH 3/13] CLK: TI: DRA7: " Julia Lawall
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: John W. Linville
  Cc: kernel-janitors, libertas-dev, linux-wireless, netdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Alternatively, was an error code intended in the bad length case, as is
done in process_brxed_802_11_packet?

 drivers/net/wireless/libertas/rx.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
index c7366b0..807c5b8 100644
--- a/drivers/net/wireless/libertas/rx.c
+++ b/drivers/net/wireless/libertas/rx.c
@@ -55,7 +55,6 @@ static int process_rxed_802_11_packet(struct lbs_private *priv,
  */
 int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
 {
-	int ret = 0;
 	struct net_device *dev = priv->dev;
 	struct rxpackethdr *p_rx_pkt;
 	struct rxpd *p_rx_pd;
@@ -86,7 +85,6 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
 	if (skb->len < (ETH_HLEN + 8 + sizeof(struct rxpd))) {
 		lbs_deb_rx("rx err: frame received with bad length\n");
 		dev->stats.rx_length_errors++;
-		ret = 0;
 		dev_kfree_skb(skb);
 		goto done;
 	}
@@ -154,10 +152,9 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
 	else
 		netif_rx_ni(skb);
 
-	ret = 0;
 done:
-	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", ret);
-	return ret;
+	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", 0);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(lbs_process_rxed_packet);
 


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

* [PATCH 3/13] CLK: TI: DRA7: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
  2014-05-19  4:31 ` [PATCH 1/13] libertas: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  7:41   ` Dan Carpenter
  2014-05-19  4:31 ` [PATCH 4/13] wimax/i2400m: make return of 0 explicit Julia Lawall
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Mike Turquette; +Cc: kernel-janitors, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

Additionally dropped some unneeded braces in an affected if.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Alternatively, is an error code wanted in either of the MAX_APLL_WAIT_TRIES
cases?

 drivers/clk/ti/apll.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index b986f61..659032a 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -37,7 +37,7 @@
 static int dra7_apll_enable(struct clk_hw *hw)
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
-	int r = 0, i = 0;
+	int i = 0;
 	struct dpll_data *ad;
 	const char *clk_name;
 	u8 state = 1;
@@ -55,7 +55,7 @@ static int dra7_apll_enable(struct clk_hw *hw)
 	v = ti_clk_ll_ops->clk_readl(ad->idlest_reg);
 
 	if ((v & ad->idlest_mask) == state)
-		return r;
+		return 0;
 
 	v = ti_clk_ll_ops->clk_readl(ad->control_reg);
 	v &= ~ad->enable_mask;
@@ -74,17 +74,14 @@ static int dra7_apll_enable(struct clk_hw *hw)
 		udelay(1);
 	}
 
-	if (i == MAX_APLL_WAIT_TRIES) {
+	if (i == MAX_APLL_WAIT_TRIES)
 		pr_warn("clock: %s failed transition to '%s'\n",
 			clk_name, (state) ? "locked" : "bypassed");
-	} else {
+	else
 		pr_debug("clock: %s transition to '%s' in %d loops\n",
 			 clk_name, (state) ? "locked" : "bypassed", i);
 
-		r = 0;
-	}
-
-	return r;
+	return 0;
 }
 
 static void dra7_apll_disable(struct clk_hw *hw)


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

* [PATCH 4/13] wimax/i2400m: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
  2014-05-19  4:31 ` [PATCH 1/13] libertas: " Julia Lawall
  2014-05-19  4:31 ` [PATCH 3/13] CLK: TI: DRA7: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  7:47   ` walter harms
  2014-05-19 12:46   ` Sergei Shtylyov
  2014-05-19  4:31 ` [PATCH 5/13] usb: gadget: " Julia Lawall
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Inaky Perez-Gonzalez; +Cc: kernel-janitors, linux-wimax, netdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Alternatively, is an error code wanted under the if?

 drivers/net/wimax/i2400m/driver.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 9c34d2f..901a534 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -500,26 +500,23 @@ int i2400m_pm_notifier(struct notifier_block *notifier,
  */
 int i2400m_pre_reset(struct i2400m *i2400m)
 {
-	int result;
 	struct device *dev = i2400m_dev(i2400m);
 
 	d_fnstart(3, dev, "(i2400m %p)\n", i2400m);
 	d_printf(1, dev, "pre-reset shut down\n");
 
-	result = 0;
 	mutex_lock(&i2400m->init_mutex);
 	if (i2400m->updown) {
 		netif_tx_disable(i2400m->wimax_dev.net_dev);
 		__i2400m_dev_stop(i2400m);
-		result = 0;
 		/* down't set updown to zero -- this way
 		 * post_reset can restore properly */
 	}
 	mutex_unlock(&i2400m->init_mutex);
 	if (i2400m->bus_release)
 		i2400m->bus_release(i2400m);
-	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, result);
-	return result;
+	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, 0);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(i2400m_pre_reset);
 


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

* [PATCH 5/13] usb: gadget: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (2 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 4/13] wimax/i2400m: make return of 0 explicit Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  4:31 ` [PATCH 6/13] [NETFILTER]: arp_tables: " Julia Lawall
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: kernel-janitors, Greg Kroah-Hartman, linux-usb, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/usb/gadget/composite.c |    6 ++----
 drivers/usb/gadget/dummy_hcd.c |    4 +---
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 042c66b..f801519 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1312,9 +1312,7 @@ static void fill_ext_compat(struct usb_configuration *c, u8 *buf)
 static int count_ext_prop(struct usb_configuration *c, int interface)
 {
 	struct usb_function *f;
-	int j, res;
-
-	res = 0;
+	int j;
 
 	f = c->interface[interface];
 	for (j = 0; j < f->os_desc_n; ++j) {
@@ -1326,7 +1324,7 @@ static int count_ext_prop(struct usb_configuration *c, int interface)
 		if (d && d->ext_compat_id)
 			return d->ext_prop_count;
 	}
-	return res;
+	return 0;
 }
 
 static int len_ext_prop(struct usb_configuration *c, int interface)
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index 8c06430..2b54955 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -561,7 +561,6 @@ static int dummy_disable(struct usb_ep *_ep)
 	struct dummy_ep		*ep;
 	struct dummy		*dum;
 	unsigned long		flags;
-	int			retval;
 
 	ep = usb_ep_to_dummy_ep(_ep);
 	if (!_ep || !ep->desc || _ep->name == ep0name)
@@ -571,12 +570,11 @@ static int dummy_disable(struct usb_ep *_ep)
 	spin_lock_irqsave(&dum->lock, flags);
 	ep->desc = NULL;
 	ep->stream_en = 0;
-	retval = 0;
 	nuke(dum, ep);
 	spin_unlock_irqrestore(&dum->lock, flags);
 
 	dev_dbg(udc_dev(dum), "disabled %s\n", _ep->name);
-	return retval;
+	return 0;
 }
 
 static struct usb_request *dummy_alloc_request(struct usb_ep *_ep,


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

* [PATCH 6/13] [NETFILTER]: arp_tables: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (3 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 5/13] usb: gadget: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  4:31 ` [PATCH 7/13] staging: wlags49_h2: " Julia Lawall
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: kernel-janitors, Patrick McHardy, Jozsef Kadlecsik,
	David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, netfilter-devel, netfilter, coreteam, netdev,
	linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 net/ipv4/netfilter/arp_tables.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f95b6f9..89f6737 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -1289,9 +1289,8 @@ compat_copy_entry_from_user(struct compat_arpt_entry *e, void **dstptr,
 	struct xt_target *target;
 	struct arpt_entry *de;
 	unsigned int origsize;
-	int ret, h;
+	int h;
 
-	ret = 0;
 	origsize = *size;
 	de = (struct arpt_entry *)*dstptr;
 	memcpy(de, e, sizeof(struct arpt_entry));
@@ -1312,7 +1311,7 @@ compat_copy_entry_from_user(struct compat_arpt_entry *e, void **dstptr,
 		if ((unsigned char *)de - base < newinfo->underflow[h])
 			newinfo->underflow[h] -= origsize - *size;
 	}
-	return ret;
+	return 0;
 }
 
 static int translate_compat_table(const char *name,


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

* [PATCH 7/13] staging: wlags49_h2: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (4 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 6/13] [NETFILTER]: arp_tables: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  4:31 ` [PATCH 8/13] sound: mpu401.c: " Julia Lawall
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Henk de Groot; +Cc: kernel-janitors, Greg Kroah-Hartman, devel, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
I'm not sure to understand this code.  There seem to be two cases where
ret was undefined at the point of the return (on failure of the case 0
test and in the default case).

 drivers/staging/wlags49_h2/wl_wext.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wlags49_h2/wl_wext.c b/drivers/staging/wlags49_h2/wl_wext.c
index 49eeeae..3aeff81 100644
--- a/drivers/staging/wlags49_h2/wl_wext.c
+++ b/drivers/staging/wlags49_h2/wl_wext.c
@@ -159,15 +159,12 @@ static int hermes_set_tkip_keys(ltv_t *ltv, u16 key_idx, u8 *addr,
 /* Set up the LTV to clear the appropriate key */
 static int hermes_clear_tkip_keys(ltv_t *ltv, u16 key_idx, u8 *addr)
 {
-	int ret;
-
 	switch (key_idx) {
 	case 0:
 		if (!is_broadcast_ether_addr(addr)) {
 			ltv->len = 7;
 			ltv->typ = CFG_REMOVE_TKIP_MAPPED_KEY;
 			memcpy(&ltv->u.u8[0], addr, ETH_ALEN);
-			ret = 0;
 		}
 		break;
 	case 1:
@@ -178,13 +175,12 @@ static int hermes_clear_tkip_keys(ltv_t *ltv, u16 key_idx, u8 *addr)
 		ltv->typ = CFG_REMOVE_TKIP_DEFAULT_KEY;
 		ltv->u.u16[0] = cpu_to_le16(key_idx);
 
-		ret = 0;
 		break;
 	default:
 		break;
 	}
 
-	return ret;
+	return 0;
 }
 
 /* Set the WEP keys in the wl_private structure */
@@ -3027,13 +3023,10 @@ static int wireless_set_genie(struct net_device *dev,
 			      struct iw_point *data, char *extra)
 
 {
-	int   ret = 0;
-
 	/* We can't write this to the card, but apparently this
 	 * operation needs to succeed */
-	ret = 0;
 
-	return ret;
+	return 0;
 }
 /*============================================================================*/
 


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

* [PATCH 8/13] sound: mpu401.c: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (5 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 7/13] staging: wlags49_h2: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  8:11   ` Takashi Iwai
  2014-05-19  4:31 ` [PATCH 9/13] md: " Julia Lawall
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: kernel-janitors, Takashi Iwai, alsa-devel, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 sound/oss/mpu401.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/oss/mpu401.c b/sound/oss/mpu401.c
index 25e4609..3bbc3ec 100644
--- a/sound/oss/mpu401.c
+++ b/sound/oss/mpu401.c
@@ -567,7 +567,6 @@ static int mpu401_out(int dev, unsigned char midi_byte)
 static int mpu401_command(int dev, mpu_command_rec * cmd)
 {
 	int i, timeout, ok;
-	int ret = 0;
 	unsigned long   flags;
 	struct mpu_config *devc;
 
@@ -644,7 +643,6 @@ retry:
 			}
 		}
 	}
-	ret = 0;
 	cmd->data[0] = 0;
 
 	if (cmd->nr_returns)
@@ -666,7 +664,7 @@ retry:
 		}
 	}
 	spin_unlock_irqrestore(&devc->lock,flags);
-	return ret;
+	return 0;
 }
 
 static int mpu_cmd(int dev, int cmd, int data)


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

* [PATCH 9/13] md: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (6 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 8/13] sound: mpu401.c: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-20  5:45   ` NeilBrown
  2014-05-19  4:31 ` [PATCH 10/13] staging: rtl8192e: " Julia Lawall
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Neil Brown; +Cc: kernel-janitors, linux-raid, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Alternatively, was an error code intended in the MD_MAX_BADBLOCKS case?

 drivers/md/md.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index f477e4c..23b7fee 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8297,7 +8297,6 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
 	u64 *p;
 	int lo, hi;
 	sector_t target = s + sectors;
-	int rv = 0;
 
 	if (bb->shift > 0) {
 		/* When clearing we round the start up and the end down.
@@ -8339,10 +8338,8 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
 
 			if (a < s) {
 				/* we need to split this range */
-				if (bb->count >= MD_MAX_BADBLOCKS) {
-					rv = 0;
+				if (bb->count >= MD_MAX_BADBLOCKS)
 					goto out;
-				}
 				memmove(p+lo+1, p+lo, (bb->count - lo) * 8);
 				bb->count++;
 				p[lo] = BB_MAKE(a, s-a, ack);
@@ -8378,7 +8375,7 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
 	bb->changed = 1;
 out:
 	write_sequnlock_irq(&bb->lock);
-	return rv;
+	return 0;
 }
 
 int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,


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

* [PATCH 10/13] staging: rtl8192e: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (7 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 9/13] md: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  4:31 ` [PATCH 11/13] ufs: " Julia Lawall
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: kernel-janitors, devel, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary use of a local variable to immediately return 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 356d521..2920e40 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -1915,8 +1915,7 @@ int rtl8192_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
 	if (queue_index == TXCMD_QUEUE) {
 		rtl8192_tx_cmd(dev, skb);
-		ret = 0;
-		return ret;
+		return 0;
 	} else {
 		tcb_desc->RATRIndex = 7;
 		tcb_desc->bTxDisableRateFallBack = 1;


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

* [PATCH 11/13] ufs: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (8 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 10/13] staging: rtl8192e: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  4:31 ` [PATCH 12/13] brcmsmac: " Julia Lawall
  2014-05-19  4:31 ` [PATCH 13/13] s390/irq: " Julia Lawall
  11 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Evgeniy Dushistov; +Cc: kernel-janitors, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 fs/ufs/truncate.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/ufs/truncate.c b/fs/ufs/truncate.c
index f04f89f..a17f0a9 100644
--- a/fs/ufs/truncate.c
+++ b/fs/ufs/truncate.c
@@ -72,7 +72,6 @@ static int ufs_trunc_direct(struct inode *inode)
 	u64 frag1, frag2, frag3, frag4, block1, block2;
 	unsigned frag_to_free, free_count;
 	unsigned i, tmp;
-	int retry;
 	
 	UFSD("ENTER: ino %lu\n", inode->i_ino);
 
@@ -81,7 +80,6 @@ static int ufs_trunc_direct(struct inode *inode)
 	
 	frag_to_free = 0;
 	free_count = 0;
-	retry = 0;
 	
 	frag1 = DIRECT_FRAGMENT;
 	frag4 = min_t(u64, UFS_NDIR_FRAGMENT, ufsi->i_lastfrag);
@@ -164,7 +162,7 @@ next1:
  next3:
 
 	UFSD("EXIT: ino %lu\n", inode->i_ino);
-	return retry;
+	return 0;
 }
 
 
@@ -176,7 +174,6 @@ static int ufs_trunc_indirect(struct inode *inode, u64 offset, void *p)
 	void *ind;
 	u64 tmp, indirect_block, i, frag_to_free;
 	unsigned free_count;
-	int retry;
 
 	UFSD("ENTER: ino %lu, offset %llu, p: %p\n",
 	     inode->i_ino, (unsigned long long)offset, p);
@@ -188,7 +185,6 @@ static int ufs_trunc_indirect(struct inode *inode, u64 offset, void *p)
 
 	frag_to_free = 0;
 	free_count = 0;
-	retry = 0;
 	
 	tmp = ufs_data_ptr_to_cpu(sb, p);
 	if (!tmp)
@@ -248,7 +244,7 @@ static int ufs_trunc_indirect(struct inode *inode, u64 offset, void *p)
 	
 	UFSD("EXIT: ino %lu\n", inode->i_ino);
 	
-	return retry;
+	return 0;
 }
 
 static int ufs_trunc_dindirect(struct inode *inode, u64 offset, void *p)


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

* [PATCH 12/13] brcmsmac: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (9 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 11/13] ufs: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  4:31 ` [PATCH 13/13] s390/irq: " Julia Lawall
  11 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Brett Rudley
  Cc: kernel-janitors, Arend van Spriel, Franky (Zhenhui) Lin,
	Hante Meuleman, John W. Linville, linux-wireless,
	brcm80211-dev-list, netdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/net/wireless/brcm80211/brcmsmac/main.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 9417cb5..3054725 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -4875,9 +4875,6 @@ static int brcms_b_detach(struct brcms_c_info *wlc)
 	uint i;
 	struct brcms_hw_band *band;
 	struct brcms_hardware *wlc_hw = wlc->hw;
-	int callbacks;
-
-	callbacks = 0;
 
 	brcms_b_detach_dmapio(wlc_hw);
 
@@ -4901,7 +4898,7 @@ static int brcms_b_detach(struct brcms_c_info *wlc)
 		wlc_hw->sih = NULL;
 	}
 
-	return callbacks;
+	return 0;
 
 }
 


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

* [PATCH 13/13] s390/irq: make return of 0 explicit
  2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
                   ` (10 preceding siblings ...)
  2014-05-19  4:31 ` [PATCH 12/13] brcmsmac: " Julia Lawall
@ 2014-05-19  4:31 ` Julia Lawall
  2014-05-19  6:03   ` Heiko Carstens
  11 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2014-05-19  4:31 UTC (permalink / raw)
  To: Robert Richter
  Cc: kernel-janitors, Martin Schwidefsky, Heiko Carstens, linux390,
	oprofile-list, linux-s390, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 arch/s390/oprofile/hwsampler.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/s390/oprofile/hwsampler.c b/arch/s390/oprofile/hwsampler.c
index 276f2e2..28b2760 100644
--- a/arch/s390/oprofile/hwsampler.c
+++ b/arch/s390/oprofile/hwsampler.c
@@ -212,10 +212,8 @@ static void init_all_cpu_buffers(void)
 static int prepare_cpu_buffers(void)
 {
 	int cpu;
-	int rc;
 	struct hws_cpu_buffer *cb;
 
-	rc = 0;
 	for_each_online_cpu(cpu) {
 		cb = &per_cpu(sampler_cpu_buffer, cpu);
 		atomic_set(&cb->ext_params, 0);
@@ -231,7 +229,7 @@ static int prepare_cpu_buffers(void)
 		cb->stop_mode = 0;
 	}
 
-	return rc;
+	return 0;
 }
 
 /*
@@ -1156,7 +1154,7 @@ int hwsampler_stop_all(void)
 	rc = 0;
 	if (hws_state == HWS_INIT) {
 		mutex_unlock(&hws_sem);
-		return rc;
+		return 0;
 	}
 	hws_state = HWS_STOPPING;
 	mutex_unlock(&hws_sem);


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

* Re: [PATCH 13/13] s390/irq: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 13/13] s390/irq: " Julia Lawall
@ 2014-05-19  6:03   ` Heiko Carstens
  0 siblings, 0 replies; 35+ messages in thread
From: Heiko Carstens @ 2014-05-19  6:03 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Robert Richter, kernel-janitors, Martin Schwidefsky, linux390,
	oprofile-list, linux-s390, linux-kernel

On Mon, May 19, 2014 at 06:31:15AM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
>  arch/s390/oprofile/hwsampler.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/oprofile/hwsampler.c b/arch/s390/oprofile/hwsampler.c
> index 276f2e2..28b2760 100644
> --- a/arch/s390/oprofile/hwsampler.c
> +++ b/arch/s390/oprofile/hwsampler.c
> @@ -212,10 +212,8 @@ static void init_all_cpu_buffers(void)
>  static int prepare_cpu_buffers(void)
>  {
>  	int cpu;
> -	int rc;
>  	struct hws_cpu_buffer *cb;
> 
> -	rc = 0;
>  	for_each_online_cpu(cpu) {
>  		cb = &per_cpu(sampler_cpu_buffer, cpu);
>  		atomic_set(&cb->ext_params, 0);
> @@ -231,7 +229,7 @@ static int prepare_cpu_buffers(void)
>  		cb->stop_mode = 0;
>  	}
> 
> -	return rc;
> +	return 0;
>  }

Thanks Julia,

I applied a slightly different version which also turns prepare_cpu_buffers
into a void returning function.


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

* Re: [PATCH 3/13] CLK: TI: DRA7: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 3/13] CLK: TI: DRA7: " Julia Lawall
@ 2014-05-19  7:41   ` Dan Carpenter
  2014-05-19  8:08     ` Tero Kristo
  0 siblings, 1 reply; 35+ messages in thread
From: Dan Carpenter @ 2014-05-19  7:41 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Mike Turquette, kernel-janitors, linux-kernel, J Keerthy, Tero Kristo

This one does feel like a bug in the original code as you mention.  I
have added the TI devs to the CC list so they can help us.

regards,
dan carpenter

On Mon, May 19, 2014 at 06:31:05AM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> Additionally dropped some unneeded braces in an affected if.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
> 
> -ret = 0;
> ... when != ret = e
> return 
> - ret
> + 0
>   ;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Alternatively, is an error code wanted in either of the MAX_APLL_WAIT_TRIES
> cases?
> 
>  drivers/clk/ti/apll.c |   13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
> index b986f61..659032a 100644
> --- a/drivers/clk/ti/apll.c
> +++ b/drivers/clk/ti/apll.c
> @@ -37,7 +37,7 @@
>  static int dra7_apll_enable(struct clk_hw *hw)
>  {
>  	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
> -	int r = 0, i = 0;
> +	int i = 0;
>  	struct dpll_data *ad;
>  	const char *clk_name;
>  	u8 state = 1;
> @@ -55,7 +55,7 @@ static int dra7_apll_enable(struct clk_hw *hw)
>  	v = ti_clk_ll_ops->clk_readl(ad->idlest_reg);
>  
>  	if ((v & ad->idlest_mask) == state)
> -		return r;
> +		return 0;
>  
>  	v = ti_clk_ll_ops->clk_readl(ad->control_reg);
>  	v &= ~ad->enable_mask;
> @@ -74,17 +74,14 @@ static int dra7_apll_enable(struct clk_hw *hw)
>  		udelay(1);
>  	}
>  
> -	if (i == MAX_APLL_WAIT_TRIES) {
> +	if (i == MAX_APLL_WAIT_TRIES)
>  		pr_warn("clock: %s failed transition to '%s'\n",
>  			clk_name, (state) ? "locked" : "bypassed");
> -	} else {
> +	else
>  		pr_debug("clock: %s transition to '%s' in %d loops\n",
>  			 clk_name, (state) ? "locked" : "bypassed", i);
>  
> -		r = 0;
> -	}
> -
> -	return r;
> +	return 0;
>  }
>  
>  static void dra7_apll_disable(struct clk_hw *hw)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/13] libertas: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 1/13] libertas: " Julia Lawall
@ 2014-05-19  7:43   ` walter harms
  2014-05-19 12:45   ` Sergei Shtylyov
  1 sibling, 0 replies; 35+ messages in thread
From: walter harms @ 2014-05-19  7:43 UTC (permalink / raw)
  To: Julia Lawall
  Cc: John W. Linville, kernel-janitors, libertas-dev, linux-wireless,
	netdev, linux-kernel



Am 19.05.2014 06:31, schrieb Julia Lawall:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
> 
> -ret = 0;
> ... when != ret = e
> return 
> - ret
> + 0
>   ;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Alternatively, was an error code intended in the bad length case, as is
> done in process_brxed_802_11_packet?
> 
>  drivers/net/wireless/libertas/rx.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
> index c7366b0..807c5b8 100644
> --- a/drivers/net/wireless/libertas/rx.c
> +++ b/drivers/net/wireless/libertas/rx.c
> @@ -55,7 +55,6 @@ static int process_rxed_802_11_packet(struct lbs_private *priv,
>   */
>  int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
>  {
> -	int ret = 0;
>  	struct net_device *dev = priv->dev;
>  	struct rxpackethdr *p_rx_pkt;
>  	struct rxpd *p_rx_pd;
> @@ -86,7 +85,6 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
>  	if (skb->len < (ETH_HLEN + 8 + sizeof(struct rxpd))) {
>  		lbs_deb_rx("rx err: frame received with bad length\n");
>  		dev->stats.rx_length_errors++;
> -		ret = 0;
>  		dev_kfree_skb(skb);
>  		goto done;
>  	}
> @@ -154,10 +152,9 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
>  	else
>  		netif_rx_ni(skb);
>  
> -	ret = 0;
>  done:
> -	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", ret);
> -	return ret;
> +	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", 0);
> +	return 0;
>  }

i guess lbs_deb_leave_args() is obsolet now.

re,
 wh


>  EXPORT_SYMBOL_GPL(lbs_process_rxed_packet);
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH 4/13] wimax/i2400m: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 4/13] wimax/i2400m: make return of 0 explicit Julia Lawall
@ 2014-05-19  7:47   ` walter harms
  2014-05-19 11:30     ` Julia Lawall
  2014-05-19 12:46   ` Sergei Shtylyov
  1 sibling, 1 reply; 35+ messages in thread
From: walter harms @ 2014-05-19  7:47 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Inaky Perez-Gonzalez, kernel-janitors, linux-wimax, netdev, linux-kernel



Am 19.05.2014 06:31, schrieb Julia Lawall:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
> 
> -ret = 0;
> ... when != ret = e
> return 
> - ret
> + 0
>   ;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Alternatively, is an error code wanted under the if?
> 
>  drivers/net/wimax/i2400m/driver.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
> index 9c34d2f..901a534 100644
> --- a/drivers/net/wimax/i2400m/driver.c
> +++ b/drivers/net/wimax/i2400m/driver.c
> @@ -500,26 +500,23 @@ int i2400m_pm_notifier(struct notifier_block *notifier,
>   */
>  int i2400m_pre_reset(struct i2400m *i2400m)
>  {
> -	int result;
>  	struct device *dev = i2400m_dev(i2400m);
>  
>  	d_fnstart(3, dev, "(i2400m %p)\n", i2400m);
>  	d_printf(1, dev, "pre-reset shut down\n");
>  
> -	result = 0;
>  	mutex_lock(&i2400m->init_mutex);
>  	if (i2400m->updown) {
>  		netif_tx_disable(i2400m->wimax_dev.net_dev);
>  		__i2400m_dev_stop(i2400m);
> -		result = 0;
>  		/* down't set updown to zero -- this way
>  		 * post_reset can restore properly */
>  	}
>  	mutex_unlock(&i2400m->init_mutex);
>  	if (i2400m->bus_release)
>  		i2400m->bus_release(i2400m);
> -	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, result);
> -	return result;
> +	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, 0);


	d_fnend(3, dev, "i2400m=%p\n", i2400m);

or something like that
re,
 wh

> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(i2400m_pre_reset);
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH 3/13] CLK: TI: DRA7: make return of 0 explicit
  2014-05-19  7:41   ` Dan Carpenter
@ 2014-05-19  8:08     ` Tero Kristo
  2014-05-19 11:25       ` [PATCH] CLK: TI: DRA7: return error code in failure case Julia Lawall
  0 siblings, 1 reply; 35+ messages in thread
From: Tero Kristo @ 2014-05-19  8:08 UTC (permalink / raw)
  To: Dan Carpenter, Julia Lawall
  Cc: Mike Turquette, kernel-janitors, linux-kernel, J Keerthy

On 05/19/2014 10:41 AM, Dan Carpenter wrote:
> This one does feel like a bug in the original code as you mention.  I
> have added the TI devs to the CC list so they can help us.

Yes this is a bug, the dra7_apll_enable() should return some sort of 
error code if the lock fails. EBUSY maybe?

-Tero

>
> regards,
> dan carpenter
>
> On Mon, May 19, 2014 at 06:31:05AM +0200, Julia Lawall wrote:
>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>>
>> Delete unnecessary local variable whose value is always 0 and that hides
>> the fact that the result is always 0.
>>
>> Additionally dropped some unneeded braces in an affected if.
>>
>> A simplified version of the semantic patch that fixes this problem is as
>> follows: (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @r exists@
>> local idexpression ret;
>> expression e;
>> position p;
>> @@
>>
>> -ret = 0;
>> ... when != ret = e
>> return
>> - ret
>> + 0
>>    ;
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>
>> ---
>> Alternatively, is an error code wanted in either of the MAX_APLL_WAIT_TRIES
>> cases?
>>
>>   drivers/clk/ti/apll.c |   13 +++++--------
>>   1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
>> index b986f61..659032a 100644
>> --- a/drivers/clk/ti/apll.c
>> +++ b/drivers/clk/ti/apll.c
>> @@ -37,7 +37,7 @@
>>   static int dra7_apll_enable(struct clk_hw *hw)
>>   {
>>   	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
>> -	int r = 0, i = 0;
>> +	int i = 0;
>>   	struct dpll_data *ad;
>>   	const char *clk_name;
>>   	u8 state = 1;
>> @@ -55,7 +55,7 @@ static int dra7_apll_enable(struct clk_hw *hw)
>>   	v = ti_clk_ll_ops->clk_readl(ad->idlest_reg);
>>
>>   	if ((v & ad->idlest_mask) == state)
>> -		return r;
>> +		return 0;
>>
>>   	v = ti_clk_ll_ops->clk_readl(ad->control_reg);
>>   	v &= ~ad->enable_mask;
>> @@ -74,17 +74,14 @@ static int dra7_apll_enable(struct clk_hw *hw)
>>   		udelay(1);
>>   	}
>>
>> -	if (i == MAX_APLL_WAIT_TRIES) {
>> +	if (i == MAX_APLL_WAIT_TRIES)
>>   		pr_warn("clock: %s failed transition to '%s'\n",
>>   			clk_name, (state) ? "locked" : "bypassed");
>> -	} else {
>> +	else
>>   		pr_debug("clock: %s transition to '%s' in %d loops\n",
>>   			 clk_name, (state) ? "locked" : "bypassed", i);
>>
>> -		r = 0;
>> -	}
>> -
>> -	return r;
>> +	return 0;
>>   }
>>
>>   static void dra7_apll_disable(struct clk_hw *hw)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH 8/13] sound: mpu401.c: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 8/13] sound: mpu401.c: " Julia Lawall
@ 2014-05-19  8:11   ` Takashi Iwai
  0 siblings, 0 replies; 35+ messages in thread
From: Takashi Iwai @ 2014-05-19  8:11 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Jaroslav Kysela, kernel-janitors, alsa-devel, linux-kernel

At Mon, 19 May 2014 06:31:10 +0200,
Julia Lawall wrote:
> 
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
> 
> -ret = 0;
> ... when != ret = e
> return 
> - ret
> + 0
>   ;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Thanks, applied.


Takashi

> 
> ---
>  sound/oss/mpu401.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/sound/oss/mpu401.c b/sound/oss/mpu401.c
> index 25e4609..3bbc3ec 100644
> --- a/sound/oss/mpu401.c
> +++ b/sound/oss/mpu401.c
> @@ -567,7 +567,6 @@ static int mpu401_out(int dev, unsigned char midi_byte)
>  static int mpu401_command(int dev, mpu_command_rec * cmd)
>  {
>  	int i, timeout, ok;
> -	int ret = 0;
>  	unsigned long   flags;
>  	struct mpu_config *devc;
>  
> @@ -644,7 +643,6 @@ retry:
>  			}
>  		}
>  	}
> -	ret = 0;
>  	cmd->data[0] = 0;
>  
>  	if (cmd->nr_returns)
> @@ -666,7 +664,7 @@ retry:
>  		}
>  	}
>  	spin_unlock_irqrestore(&devc->lock,flags);
> -	return ret;
> +	return 0;
>  }
>  
>  static int mpu_cmd(int dev, int cmd, int data)
> 

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

* [PATCH] CLK: TI: DRA7: return error code in failure case
  2014-05-19  8:08     ` Tero Kristo
@ 2014-05-19 11:25       ` Julia Lawall
  2014-05-19 12:23         ` Tero Kristo
  0 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2014-05-19 11:25 UTC (permalink / raw)
  To: Tero Kristo
  Cc: Dan Carpenter, Mike Turquette, kernel-janitors, linux-kernel, J Keerthy

From: Julia Lawall <Julia.Lawall@lip6.fr>

Add a returned error code in the MAX_APLL_WAIT_TRIES case.  Remove the
updating of the return variable r to 0 if MAX_APLL_WAIT_TRIES is not yet
reached, because r is already 0 at this point.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index b986f61..efc71a0 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -77,13 +77,11 @@ static int dra7_apll_enable(struct clk_hw *hw)
 	if (i == MAX_APLL_WAIT_TRIES) {
 		pr_warn("clock: %s failed transition to '%s'\n",
 			clk_name, (state) ? "locked" : "bypassed");
-	} else {
+		r = -EBUSY;
+	} else
 		pr_debug("clock: %s transition to '%s' in %d loops\n",
 			 clk_name, (state) ? "locked" : "bypassed", i);

-		r = 0;
-	}
-
 	return r;
 }


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

* Re: [PATCH 4/13] wimax/i2400m: make return of 0 explicit
  2014-05-19  7:47   ` walter harms
@ 2014-05-19 11:30     ` Julia Lawall
  2014-05-19 13:22       ` walter harms
  0 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2014-05-19 11:30 UTC (permalink / raw)
  To: walter harms
  Cc: Inaky Perez-Gonzalez, kernel-janitors, linux-wimax, netdev, linux-kernel



On Mon, 19 May 2014, walter harms wrote:

>
>
> Am 19.05.2014 06:31, schrieb Julia Lawall:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > Delete unnecessary local variable whose value is always 0 and that hides
> > the fact that the result is always 0.
> >
> > A simplified version of the semantic patch that fixes this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @r exists@
> > local idexpression ret;
> > expression e;
> > position p;
> > @@
> >
> > -ret = 0;
> > ... when != ret = e
> > return
> > - ret
> > + 0
> >   ;
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> > Alternatively, is an error code wanted under the if?
> >
> >  drivers/net/wimax/i2400m/driver.c |    7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
> > index 9c34d2f..901a534 100644
> > --- a/drivers/net/wimax/i2400m/driver.c
> > +++ b/drivers/net/wimax/i2400m/driver.c
> > @@ -500,26 +500,23 @@ int i2400m_pm_notifier(struct notifier_block *notifier,
> >   */
> >  int i2400m_pre_reset(struct i2400m *i2400m)
> >  {
> > -	int result;
> >  	struct device *dev = i2400m_dev(i2400m);
> >
> >  	d_fnstart(3, dev, "(i2400m %p)\n", i2400m);
> >  	d_printf(1, dev, "pre-reset shut down\n");
> >
> > -	result = 0;
> >  	mutex_lock(&i2400m->init_mutex);
> >  	if (i2400m->updown) {
> >  		netif_tx_disable(i2400m->wimax_dev.net_dev);
> >  		__i2400m_dev_stop(i2400m);
> > -		result = 0;
> >  		/* down't set updown to zero -- this way
> >  		 * post_reset can restore properly */
> >  	}
> >  	mutex_unlock(&i2400m->init_mutex);
> >  	if (i2400m->bus_release)
> >  		i2400m->bus_release(i2400m);
> > -	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, result);
> > -	return result;
> > +	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, 0);
>
>
> 	d_fnend(3, dev, "i2400m=%p\n", i2400m);

All the other logging messages in this file seem to have the form
(i2400m %p) = %d
or (i2400m %p) = void in the case of a function that has no return value.
I don't know if a return value is wanted here, but since the function is
exported, perhaps it is best not to change its type.

julia

> or something like that
> re,
>  wh
>
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(i2400m_pre_reset);
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
>

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

* Re: [PATCH] CLK: TI: DRA7: return error code in failure case
  2014-05-19 11:25       ` [PATCH] CLK: TI: DRA7: return error code in failure case Julia Lawall
@ 2014-05-19 12:23         ` Tero Kristo
  2014-05-28 21:47           ` Mike Turquette
  0 siblings, 1 reply; 35+ messages in thread
From: Tero Kristo @ 2014-05-19 12:23 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Dan Carpenter, Mike Turquette, kernel-janitors, linux-kernel, J Keerthy

On 05/19/2014 02:25 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Add a returned error code in the MAX_APLL_WAIT_TRIES case.  Remove the
> updating of the return variable r to 0 if MAX_APLL_WAIT_TRIES is not yet
> reached, because r is already 0 at this point.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Acked-by: Tero Kristo <t-kristo@ti.com>

Mike, do you want to queue this as a fix or shall I add this to be 
queued for 3.16?

-Tero

>
> ---
> diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
> index b986f61..efc71a0 100644
> --- a/drivers/clk/ti/apll.c
> +++ b/drivers/clk/ti/apll.c
> @@ -77,13 +77,11 @@ static int dra7_apll_enable(struct clk_hw *hw)
>   	if (i == MAX_APLL_WAIT_TRIES) {
>   		pr_warn("clock: %s failed transition to '%s'\n",
>   			clk_name, (state) ? "locked" : "bypassed");
> -	} else {
> +		r = -EBUSY;
> +	} else
>   		pr_debug("clock: %s transition to '%s' in %d loops\n",
>   			 clk_name, (state) ? "locked" : "bypassed", i);
>
> -		r = 0;
> -	}
> -
>   	return r;
>   }
>


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

* Re: [PATCH 1/13] libertas: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 1/13] libertas: " Julia Lawall
  2014-05-19  7:43   ` walter harms
@ 2014-05-19 12:45   ` Sergei Shtylyov
  2014-05-19 17:25     ` Dan Williams
                       ` (2 more replies)
  1 sibling, 3 replies; 35+ messages in thread
From: Sergei Shtylyov @ 2014-05-19 12:45 UTC (permalink / raw)
  To: Julia Lawall, John W. Linville
  Cc: kernel-janitors, libertas-dev, linux-wireless, netdev, linux-kernel

Hello.

On 19-05-2014 8:31, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>

> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.

> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)

> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
>
> -ret = 0;
> ... when != ret = e
> return
> - ret
> + 0
>    ;
> // </smpl>

> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

> ---
> Alternatively, was an error code intended in the bad length case, as is
> done in process_brxed_802_11_packet?

>   drivers/net/wireless/libertas/rx.c |    7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)

> diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
> index c7366b0..807c5b8 100644
> --- a/drivers/net/wireless/libertas/rx.c
> +++ b/drivers/net/wireless/libertas/rx.c
[...]
> @@ -154,10 +152,9 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
>   	else
>   		netif_rx_ni(skb);
>
> -	ret = 0;
>   done:
> -	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", ret);
> -	return ret;
> +	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", 0);

    Why not just "ret 0"?

> +	return 0;

WBR, Sergei


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

* Re: [PATCH 4/13] wimax/i2400m: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 4/13] wimax/i2400m: make return of 0 explicit Julia Lawall
  2014-05-19  7:47   ` walter harms
@ 2014-05-19 12:46   ` Sergei Shtylyov
  2014-05-20  0:44     ` [PATCH 4/13 v2] " Julia Lawall
  1 sibling, 1 reply; 35+ messages in thread
From: Sergei Shtylyov @ 2014-05-19 12:46 UTC (permalink / raw)
  To: Julia Lawall, Inaky Perez-Gonzalez
  Cc: kernel-janitors, linux-wimax, netdev, linux-kernel

On 19-05-2014 8:31, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>

> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.

> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)

> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
>
> -ret = 0;
> ... when != ret = e
> return
> - ret
> + 0
>    ;
> // </smpl>

> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

> ---
> Alternatively, is an error code wanted under the if?

>   drivers/net/wimax/i2400m/driver.c |    7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)

> diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
> index 9c34d2f..901a534 100644
> --- a/drivers/net/wimax/i2400m/driver.c
> +++ b/drivers/net/wimax/i2400m/driver.c
> @@ -500,26 +500,23 @@ int i2400m_pm_notifier(struct notifier_block *notifier,
>    */
>   int i2400m_pre_reset(struct i2400m *i2400m)
>   {
> -	int result;
>   	struct device *dev = i2400m_dev(i2400m);
>
>   	d_fnstart(3, dev, "(i2400m %p)\n", i2400m);
>   	d_printf(1, dev, "pre-reset shut down\n");
>
> -	result = 0;
>   	mutex_lock(&i2400m->init_mutex);
>   	if (i2400m->updown) {
>   		netif_tx_disable(i2400m->wimax_dev.net_dev);
>   		__i2400m_dev_stop(i2400m);
> -		result = 0;
>   		/* down't set updown to zero -- this way
>   		 * post_reset can restore properly */
>   	}
>   	mutex_unlock(&i2400m->init_mutex);
>   	if (i2400m->bus_release)
>   		i2400m->bus_release(i2400m);
> -	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, result);
> -	return result;
> +	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, 0);

    Again, why not just "(i2400m %p) = 0\n"?

> +	return 0;

WBR, Sergei


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

* Re: [PATCH 4/13] wimax/i2400m: make return of 0 explicit
  2014-05-19 11:30     ` Julia Lawall
@ 2014-05-19 13:22       ` walter harms
  0 siblings, 0 replies; 35+ messages in thread
From: walter harms @ 2014-05-19 13:22 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Inaky Perez-Gonzalez, kernel-janitors, linux-wimax, netdev, linux-kernel



Am 19.05.2014 13:30, schrieb Julia Lawall:
> 
> 
> On Mon, 19 May 2014, walter harms wrote:
> 
>>
>>
>> Am 19.05.2014 06:31, schrieb Julia Lawall:
>>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> Delete unnecessary local variable whose value is always 0 and that hides
>>> the fact that the result is always 0.
>>>
>>> A simplified version of the semantic patch that fixes this problem is as
>>> follows: (http://coccinelle.lip6.fr/)
>>>
>>> // <smpl>
>>> @r exists@
>>> local idexpression ret;
>>> expression e;
>>> position p;
>>> @@
>>>
>>> -ret = 0;
>>> ... when != ret = e
>>> return
>>> - ret
>>> + 0
>>>   ;
>>> // </smpl>
>>>
>>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> ---
>>> Alternatively, is an error code wanted under the if?
>>>
>>>  drivers/net/wimax/i2400m/driver.c |    7 ++-----
>>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
>>> index 9c34d2f..901a534 100644
>>> --- a/drivers/net/wimax/i2400m/driver.c
>>> +++ b/drivers/net/wimax/i2400m/driver.c
>>> @@ -500,26 +500,23 @@ int i2400m_pm_notifier(struct notifier_block *notifier,
>>>   */
>>>  int i2400m_pre_reset(struct i2400m *i2400m)
>>>  {
>>> -	int result;
>>>  	struct device *dev = i2400m_dev(i2400m);
>>>
>>>  	d_fnstart(3, dev, "(i2400m %p)\n", i2400m);
>>>  	d_printf(1, dev, "pre-reset shut down\n");
>>>
>>> -	result = 0;
>>>  	mutex_lock(&i2400m->init_mutex);
>>>  	if (i2400m->updown) {
>>>  		netif_tx_disable(i2400m->wimax_dev.net_dev);
>>>  		__i2400m_dev_stop(i2400m);
>>> -		result = 0;
>>>  		/* down't set updown to zero -- this way
>>>  		 * post_reset can restore properly */
>>>  	}
>>>  	mutex_unlock(&i2400m->init_mutex);
>>>  	if (i2400m->bus_release)
>>>  		i2400m->bus_release(i2400m);
>>> -	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, result);
>>> -	return result;
>>> +	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, 0);
>>
>>
>> 	d_fnend(3, dev, "i2400m=%p\n", i2400m);
> 
> All the other logging messages in this file seem to have the form
> (i2400m %p) = %d
> or (i2400m %p) = void in the case of a function that has no return value.
> I don't know if a return value is wanted here, but since the function is
> exported, perhaps it is best not to change its type.


I got my "inspiration" from d_fnstart().
As i see it the whole function is void so printing a debug msg with a bogus
return value may confuse some people.

re,
 wh


> julia
> 
>> or something like that
>> re,
>>  wh
>>
>>> +	return 0;
>>>  }
>>>  EXPORT_SYMBOL_GPL(i2400m_pre_reset);
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
> 

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

* Re: [PATCH 1/13] libertas: make return of 0 explicit
  2014-05-19 12:45   ` Sergei Shtylyov
@ 2014-05-19 17:25     ` Dan Williams
  2014-05-22 12:32       ` [PATCH] libertas: fix return value when processing invalid packet Dan Williams
  2014-05-20  0:30     ` [PATCH 1/13] libertas: make return of 0 explicit Julia Lawall
  2014-05-20  0:36     ` Julia Lawall
  2 siblings, 1 reply; 35+ messages in thread
From: Dan Williams @ 2014-05-19 17:25 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Julia Lawall, John W. Linville, kernel-janitors, libertas-dev,
	linux-wireless, netdev, linux-kernel

On Mon, 2014-05-19 at 16:45 +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 19-05-2014 8:31, Julia Lawall wrote:
> 
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> > Delete unnecessary local variable whose value is always 0 and that hides
> > the fact that the result is always 0.
> 
> > A simplified version of the semantic patch that fixes this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> 
> > // <smpl>
> > @r exists@
> > local idexpression ret;
> > expression e;
> > position p;
> > @@
> >
> > -ret = 0;
> > ... when != ret = e
> > return
> > - ret
> > + 0
> >    ;
> > // </smpl>
> 
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> > ---
> > Alternatively, was an error code intended in the bad length case, as is
> > done in process_brxed_802_11_packet?
> 
> >   drivers/net/wireless/libertas/rx.c |    7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> > diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
> > index c7366b0..807c5b8 100644
> > --- a/drivers/net/wireless/libertas/rx.c
> > +++ b/drivers/net/wireless/libertas/rx.c
> [...]
> > @@ -154,10 +152,9 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
> >   	else
> >   		netif_rx_ni(skb);
> >
> > -	ret = 0;
> >   done:
> > -	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", ret);
> > -	return ret;
> > +	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", 0);
> 
>     Why not just "ret 0"?

I think instead we want:

diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
index c7366b0..e446fed 100644
--- a/drivers/net/wireless/libertas/rx.c
+++ b/drivers/net/wireless/libertas/rx.c
@@ -67,30 +67,32 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
 
 	lbs_deb_enter(LBS_DEB_RX);
 
 	BUG_ON(!skb);
 
 	skb->ip_summed = CHECKSUM_NONE;
 
-	if (priv->wdev->iftype == NL80211_IFTYPE_MONITOR)
-		return process_rxed_802_11_packet(priv, skb);
+	if (priv->wdev->iftype == NL80211_IFTYPE_MONITOR) {
+		ret = process_rxed_802_11_packet(priv, skb);
+		goto done;
+	}
 
 	p_rx_pd = (struct rxpd *) skb->data;
 	p_rx_pkt = (struct rxpackethdr *) ((u8 *)p_rx_pd +
 		le32_to_cpu(p_rx_pd->pkt_ptr));
 
 	dev = lbs_mesh_set_dev(priv, dev, p_rx_pd);
 
 	lbs_deb_hex(LBS_DEB_RX, "RX Data: Before chop rxpd", skb->data,
 		 min_t(unsigned int, skb->len, 100));
 
 	if (skb->len < (ETH_HLEN + 8 + sizeof(struct rxpd))) {
 		lbs_deb_rx("rx err: frame received with bad length\n");
 		dev->stats.rx_length_errors++;
-		ret = 0;
+		ret = -EINVAL;
 		dev_kfree_skb(skb);
 		goto done;
 	}
 
 	lbs_deb_rx("rx data: skb->len - pkt_ptr = %d-%zd = %zd\n",
 		skb->len, (size_t)le32_to_cpu(p_rx_pd->pkt_ptr),
 		skb->len - (size_t)le32_to_cpu(p_rx_pd->pkt_ptr));

Since that (a) preserves the enter/leave balance when monitor mode is
enabled, and (b) fixes the return code that has been 0 since the driver
was added to the tree in early 2007.  Dave Woodhouse fixed the
process_rxed_802_11_packet() return code for EINVAL case in commit
7bf02c29 from late 2007, and I think the normal codepath should do
EINVAL too.  (though it turns out nothing cares about the return code
anyway, we should still fix it)

Dan



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

* Re: [PATCH 1/13] libertas: make return of 0 explicit
  2014-05-19 12:45   ` Sergei Shtylyov
  2014-05-19 17:25     ` Dan Williams
@ 2014-05-20  0:30     ` Julia Lawall
  2014-05-20  0:36     ` Julia Lawall
  2 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-20  0:30 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: John W. Linville, kernel-janitors, libertas-dev, linux-wireless,
	netdev, linux-kernel



On Mon, 19 May 2014, Sergei Shtylyov wrote:

> Hello.
>
> On 19-05-2014 8:31, Julia Lawall wrote:
>
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> > Delete unnecessary local variable whose value is always 0 and that hides
> > the fact that the result is always 0.
>
> > A simplified version of the semantic patch that fixes this problem is as
> > follows: (http://coccinelle.lip6.fr/)
>
> > // <smpl>
> > @r exists@
> > local idexpression ret;
> > expression e;
> > position p;
> > @@
> >
> > -ret = 0;
> > ... when != ret = e
> > return
> > - ret
> > + 0
> >    ;
> > // </smpl>
>
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> > ---
> > Alternatively, was an error code intended in the bad length case, as is
> > done in process_brxed_802_11_packet?
>
> >   drivers/net/wireless/libertas/rx.c |    7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
>
> > diff --git a/drivers/net/wireless/libertas/rx.c
> > b/drivers/net/wireless/libertas/rx.c
> > index c7366b0..807c5b8 100644
> > --- a/drivers/net/wireless/libertas/rx.c
> > +++ b/drivers/net/wireless/libertas/rx.c
> [...]
> > @@ -154,10 +152,9 @@ int lbs_process_rxed_packet(struct lbs_private *priv,
> > struct sk_buff *skb)
> >   	else
> >   		netif_rx_ni(skb);
> >
> > -	ret = 0;
> >   done:
> > -	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", ret);
> > -	return ret;
> > +	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", 0);
>
>    Why not just "ret 0"?

OK, I wasn't sure if it was useful to keep the same number of arguments.
But I will update it, since from the definition of lbs_deb_leave_args it
seems ok.

julia

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

* Re: [PATCH 1/13] libertas: make return of 0 explicit
  2014-05-19 12:45   ` Sergei Shtylyov
  2014-05-19 17:25     ` Dan Williams
  2014-05-20  0:30     ` [PATCH 1/13] libertas: make return of 0 explicit Julia Lawall
@ 2014-05-20  0:36     ` Julia Lawall
  2 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2014-05-20  0:36 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: John W. Linville, kernel-janitors, libertas-dev, linux-wireless,
	netdev, linux-kernel



On Mon, 19 May 2014, Sergei Shtylyov wrote:

> Hello.
>
> On 19-05-2014 8:31, Julia Lawall wrote:
>
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> > Delete unnecessary local variable whose value is always 0 and that hides
> > the fact that the result is always 0.
>
> > A simplified version of the semantic patch that fixes this problem is as
> > follows: (http://coccinelle.lip6.fr/)
>
> > // <smpl>
> > @r exists@
> > local idexpression ret;
> > expression e;
> > position p;
> > @@
> >
> > -ret = 0;
> > ... when != ret = e
> > return
> > - ret
> > + 0
> >    ;
> > // </smpl>
>
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> > ---
> > Alternatively, was an error code intended in the bad length case, as is
> > done in process_brxed_802_11_packet?
>
> >   drivers/net/wireless/libertas/rx.c |    7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
>
> > diff --git a/drivers/net/wireless/libertas/rx.c
> > b/drivers/net/wireless/libertas/rx.c
> > index c7366b0..807c5b8 100644
> > --- a/drivers/net/wireless/libertas/rx.c
> > +++ b/drivers/net/wireless/libertas/rx.c
> [...]
> > @@ -154,10 +152,9 @@ int lbs_process_rxed_packet(struct lbs_private *priv,
> > struct sk_buff *skb)
> >   	else
> >   		netif_rx_ni(skb);
> >
> > -	ret = 0;
> >   done:
> > -	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", ret);
> > -	return ret;
> > +	lbs_deb_leave_args(LBS_DEB_RX, "ret %d", 0);
>
>    Why not just "ret 0"?

Oops, I won't make any change here, because Dan Williams has proposed
another patch that makes the ret variable useful.

julia

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

* [PATCH 4/13 v2] wimax/i2400m: make return of 0 explicit
  2014-05-19 12:46   ` Sergei Shtylyov
@ 2014-05-20  0:44     ` Julia Lawall
  2014-05-21 21:19       ` David Miller
  0 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2014-05-20  0:44 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Inaky Perez-Gonzalez, kernel-janitors, linux-wimax, netdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return
- ret
+ 0
  ;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Alternatively, is an error code wanted under the if?

v2: remove the use of 0 as an argument in the call to d_fnend

 drivers/net/wimax/i2400m/driver.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 9c34d2f..9c78090 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -500,26 +500,23 @@ int i2400m_pm_notifier(struct notifier_block *notifier,
  */
 int i2400m_pre_reset(struct i2400m *i2400m)
 {
-	int result;
 	struct device *dev = i2400m_dev(i2400m);

 	d_fnstart(3, dev, "(i2400m %p)\n", i2400m);
 	d_printf(1, dev, "pre-reset shut down\n");

-	result = 0;
 	mutex_lock(&i2400m->init_mutex);
 	if (i2400m->updown) {
 		netif_tx_disable(i2400m->wimax_dev.net_dev);
 		__i2400m_dev_stop(i2400m);
-		result = 0;
 		/* down't set updown to zero -- this way
 		 * post_reset can restore properly */
 	}
 	mutex_unlock(&i2400m->init_mutex);
 	if (i2400m->bus_release)
 		i2400m->bus_release(i2400m);
-	d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, result);
-	return result;
+	d_fnend(3, dev, "(i2400m %p) = 0\n", i2400m);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(i2400m_pre_reset);


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

* Re: [PATCH 9/13] md: make return of 0 explicit
  2014-05-19  4:31 ` [PATCH 9/13] md: " Julia Lawall
@ 2014-05-20  5:45   ` NeilBrown
  0 siblings, 0 replies; 35+ messages in thread
From: NeilBrown @ 2014-05-20  5:45 UTC (permalink / raw)
  To: Julia Lawall; +Cc: kernel-janitors, linux-raid, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

On Mon, 19 May 2014 06:31:11 +0200 Julia Lawall <Julia.Lawall@lip6.fr> wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
> 
> -ret = 0;
> ... when != ret = e
> return 
> - ret
> + 0
>   ;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Alternatively, was an error code intended in the MD_MAX_BADBLOCKS case?

Yes, I did want an error code in that case  - thanks!
I'll make a patch.

NeilBrown


> 
>  drivers/md/md.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index f477e4c..23b7fee 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8297,7 +8297,6 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
>  	u64 *p;
>  	int lo, hi;
>  	sector_t target = s + sectors;
> -	int rv = 0;
>  
>  	if (bb->shift > 0) {
>  		/* When clearing we round the start up and the end down.
> @@ -8339,10 +8338,8 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
>  
>  			if (a < s) {
>  				/* we need to split this range */
> -				if (bb->count >= MD_MAX_BADBLOCKS) {
> -					rv = 0;
> +				if (bb->count >= MD_MAX_BADBLOCKS)
>  					goto out;
> -				}
>  				memmove(p+lo+1, p+lo, (bb->count - lo) * 8);
>  				bb->count++;
>  				p[lo] = BB_MAKE(a, s-a, ack);
> @@ -8378,7 +8375,7 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
>  	bb->changed = 1;
>  out:
>  	write_sequnlock_irq(&bb->lock);
> -	return rv;
> +	return 0;
>  }
>  
>  int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

* Re: [PATCH 4/13 v2] wimax/i2400m: make return of 0 explicit
  2014-05-20  0:44     ` [PATCH 4/13 v2] " Julia Lawall
@ 2014-05-21 21:19       ` David Miller
  0 siblings, 0 replies; 35+ messages in thread
From: David Miller @ 2014-05-21 21:19 UTC (permalink / raw)
  To: julia.lawall
  Cc: sergei.shtylyov, inaky.perez-gonzalez, kernel-janitors,
	linux-wimax, netdev, linux-kernel

From: Julia Lawall <julia.lawall@lip6.fr>
Date: Tue, 20 May 2014 08:44:49 +0800 (SGT)

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Alternatively, is an error code wanted under the if?
> 
> v2: remove the use of 0 as an argument in the call to d_fnend

Applied to net-next, thanks.

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

* [PATCH] libertas: fix return value when processing invalid packet
  2014-05-19 17:25     ` Dan Williams
@ 2014-05-22 12:32       ` Dan Williams
  2014-05-22 21:57         ` James Cameron
  0 siblings, 1 reply; 35+ messages in thread
From: Dan Williams @ 2014-05-22 12:32 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: kernel-janitors, libertas-dev, netdev, linux-wireless,
	John W. Linville, linux-kernel, Julia Lawall

Nothing actually uses the return value yet, but we might as well
make it correct, like process_rxed_802_11_packet() does for the
same case.  Also ensure that if monitor mode is enabled (and
thus process_rxed_802_11_packet() is called) that the debugging
enter/leave functions are balanced.

Signed-off-by: Dan Williams <dcbw@redhat.com>
---
 drivers/net/wireless/libertas/Makefile | 4 ++--
 drivers/net/wireless/libertas/rx.c     | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
index c7366b0..e446fed 100644
--- a/drivers/net/wireless/libertas/rx.c
+++ b/drivers/net/wireless/libertas/rx.c
@@ -67,30 +67,32 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
 
 	lbs_deb_enter(LBS_DEB_RX);
 
 	BUG_ON(!skb);
 
 	skb->ip_summed = CHECKSUM_NONE;
 
-	if (priv->wdev->iftype == NL80211_IFTYPE_MONITOR)
-		return process_rxed_802_11_packet(priv, skb);
+	if (priv->wdev->iftype == NL80211_IFTYPE_MONITOR) {
+		ret = process_rxed_802_11_packet(priv, skb);
+		goto done;
+	}
 
 	p_rx_pd = (struct rxpd *) skb->data;
 	p_rx_pkt = (struct rxpackethdr *) ((u8 *)p_rx_pd +
 		le32_to_cpu(p_rx_pd->pkt_ptr));
 
 	dev = lbs_mesh_set_dev(priv, dev, p_rx_pd);
 
 	lbs_deb_hex(LBS_DEB_RX, "RX Data: Before chop rxpd", skb->data,
 		 min_t(unsigned int, skb->len, 100));
 
 	if (skb->len < (ETH_HLEN + 8 + sizeof(struct rxpd))) {
 		lbs_deb_rx("rx err: frame received with bad length\n");
 		dev->stats.rx_length_errors++;
-		ret = 0;
+		ret = -EINVAL;
 		dev_kfree_skb(skb);
 		goto done;
 	}
 
 	lbs_deb_rx("rx data: skb->len - pkt_ptr = %d-%zd = %zd\n",
 		skb->len, (size_t)le32_to_cpu(p_rx_pd->pkt_ptr),
 		skb->len - (size_t)le32_to_cpu(p_rx_pd->pkt_ptr));
-- 
1.9.0



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

* Re: [PATCH] libertas: fix return value when processing invalid packet
  2014-05-22 12:32       ` [PATCH] libertas: fix return value when processing invalid packet Dan Williams
@ 2014-05-22 21:57         ` James Cameron
  0 siblings, 0 replies; 35+ messages in thread
From: James Cameron @ 2014-05-22 21:57 UTC (permalink / raw)
  To: Dan Williams
  Cc: Sergei Shtylyov, kernel-janitors, libertas-dev, netdev,
	linux-wireless, John W. Linville, linux-kernel, Julia Lawall

On Thu, May 22, 2014 at 07:32:41AM -0500, Dan Williams wrote:
> Nothing actually uses the return value yet, but we might as well
> make it correct, like process_rxed_802_11_packet() does for the
> same case.  Also ensure that if monitor mode is enabled (and
> thus process_rxed_802_11_packet() is called) that the debugging
> enter/leave functions are balanced.
> 
> Signed-off-by: Dan Williams <dcbw@redhat.com>

Reviewed-by: James Cameron <quozl@laptop.org>

-- 
James Cameron
http://quozl.linux.org.au/

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

* Re: [PATCH] CLK: TI: DRA7: return error code in failure case
  2014-05-19 12:23         ` Tero Kristo
@ 2014-05-28 21:47           ` Mike Turquette
  2014-06-19 11:24             ` Tero Kristo
  0 siblings, 1 reply; 35+ messages in thread
From: Mike Turquette @ 2014-05-28 21:47 UTC (permalink / raw)
  To: Tero Kristo, Julia Lawall
  Cc: Dan Carpenter, kernel-janitors, linux-kernel, J Keerthy

Quoting Tero Kristo (2014-05-19 05:23:10)
> On 05/19/2014 02:25 PM, Julia Lawall wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > Add a returned error code in the MAX_APLL_WAIT_TRIES case.  Remove the
> > updating of the return variable r to 0 if MAX_APLL_WAIT_TRIES is not yet
> > reached, because r is already 0 at this point.
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Acked-by: Tero Kristo <t-kristo@ti.com>
> 
> Mike, do you want to queue this as a fix or shall I add this to be 
> queued for 3.16?

It's not a visible regression, so let's go with 3.16.

Regards,
Mike

> 
> -Tero
> 
> >
> > ---
> > diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
> > index b986f61..efc71a0 100644
> > --- a/drivers/clk/ti/apll.c
> > +++ b/drivers/clk/ti/apll.c
> > @@ -77,13 +77,11 @@ static int dra7_apll_enable(struct clk_hw *hw)
> >       if (i == MAX_APLL_WAIT_TRIES) {
> >               pr_warn("clock: %s failed transition to '%s'\n",
> >                       clk_name, (state) ? "locked" : "bypassed");
> > -     } else {
> > +             r = -EBUSY;
> > +     } else
> >               pr_debug("clock: %s transition to '%s' in %d loops\n",
> >                        clk_name, (state) ? "locked" : "bypassed", i);
> >
> > -             r = 0;
> > -     }
> > -
> >       return r;
> >   }
> >
> 

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

* Re: [PATCH] CLK: TI: DRA7: return error code in failure case
  2014-05-28 21:47           ` Mike Turquette
@ 2014-06-19 11:24             ` Tero Kristo
  0 siblings, 0 replies; 35+ messages in thread
From: Tero Kristo @ 2014-06-19 11:24 UTC (permalink / raw)
  To: Mike Turquette, Julia Lawall
  Cc: Dan Carpenter, kernel-janitors, linux-kernel, J Keerthy

On 05/29/2014 12:47 AM, Mike Turquette wrote:
> Quoting Tero Kristo (2014-05-19 05:23:10)
>> On 05/19/2014 02:25 PM, Julia Lawall wrote:
>>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> Add a returned error code in the MAX_APLL_WAIT_TRIES case.  Remove the
>>> updating of the return variable r to 0 if MAX_APLL_WAIT_TRIES is not yet
>>> reached, because r is already 0 at this point.
>>>
>>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>
>> Acked-by: Tero Kristo <t-kristo@ti.com>
>>
>> Mike, do you want to queue this as a fix or shall I add this to be
>> queued for 3.16?
>
> It's not a visible regression, so let's go with 3.16.

Thanks, queued for 3.16-rc fixes now, sorry about the latency.

-Tero

>
> Regards,
> Mike
>
>>
>> -Tero
>>
>>>
>>> ---
>>> diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
>>> index b986f61..efc71a0 100644
>>> --- a/drivers/clk/ti/apll.c
>>> +++ b/drivers/clk/ti/apll.c
>>> @@ -77,13 +77,11 @@ static int dra7_apll_enable(struct clk_hw *hw)
>>>        if (i == MAX_APLL_WAIT_TRIES) {
>>>                pr_warn("clock: %s failed transition to '%s'\n",
>>>                        clk_name, (state) ? "locked" : "bypassed");
>>> -     } else {
>>> +             r = -EBUSY;
>>> +     } else
>>>                pr_debug("clock: %s transition to '%s' in %d loops\n",
>>>                         clk_name, (state) ? "locked" : "bypassed", i);
>>>
>>> -             r = 0;
>>> -     }
>>> -
>>>        return r;
>>>    }
>>>
>>


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

end of thread, other threads:[~2014-06-19 11:24 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-19  4:31 [PATCH 0/13] make return of 0 explicit Julia Lawall
2014-05-19  4:31 ` [PATCH 1/13] libertas: " Julia Lawall
2014-05-19  7:43   ` walter harms
2014-05-19 12:45   ` Sergei Shtylyov
2014-05-19 17:25     ` Dan Williams
2014-05-22 12:32       ` [PATCH] libertas: fix return value when processing invalid packet Dan Williams
2014-05-22 21:57         ` James Cameron
2014-05-20  0:30     ` [PATCH 1/13] libertas: make return of 0 explicit Julia Lawall
2014-05-20  0:36     ` Julia Lawall
2014-05-19  4:31 ` [PATCH 3/13] CLK: TI: DRA7: " Julia Lawall
2014-05-19  7:41   ` Dan Carpenter
2014-05-19  8:08     ` Tero Kristo
2014-05-19 11:25       ` [PATCH] CLK: TI: DRA7: return error code in failure case Julia Lawall
2014-05-19 12:23         ` Tero Kristo
2014-05-28 21:47           ` Mike Turquette
2014-06-19 11:24             ` Tero Kristo
2014-05-19  4:31 ` [PATCH 4/13] wimax/i2400m: make return of 0 explicit Julia Lawall
2014-05-19  7:47   ` walter harms
2014-05-19 11:30     ` Julia Lawall
2014-05-19 13:22       ` walter harms
2014-05-19 12:46   ` Sergei Shtylyov
2014-05-20  0:44     ` [PATCH 4/13 v2] " Julia Lawall
2014-05-21 21:19       ` David Miller
2014-05-19  4:31 ` [PATCH 5/13] usb: gadget: " Julia Lawall
2014-05-19  4:31 ` [PATCH 6/13] [NETFILTER]: arp_tables: " Julia Lawall
2014-05-19  4:31 ` [PATCH 7/13] staging: wlags49_h2: " Julia Lawall
2014-05-19  4:31 ` [PATCH 8/13] sound: mpu401.c: " Julia Lawall
2014-05-19  8:11   ` Takashi Iwai
2014-05-19  4:31 ` [PATCH 9/13] md: " Julia Lawall
2014-05-20  5:45   ` NeilBrown
2014-05-19  4:31 ` [PATCH 10/13] staging: rtl8192e: " Julia Lawall
2014-05-19  4:31 ` [PATCH 11/13] ufs: " Julia Lawall
2014-05-19  4:31 ` [PATCH 12/13] brcmsmac: " Julia Lawall
2014-05-19  4:31 ` [PATCH 13/13] s390/irq: " Julia Lawall
2014-05-19  6:03   ` Heiko Carstens

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