All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Add of_node_put to avoid memory leak
@ 2010-08-31 15:48 ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Grant Likely; +Cc: kernel-janitors, linux-kernel, devicetree-discuss

These patches introduce calls to of_node_put after various functions that
call of_node_get.


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

* [PATCH 0/4] Add of_node_put to avoid memory leak
@ 2010-08-31 15:48 ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Grant Likely
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

These patches introduce calls to of_node_put after various functions that
call of_node_get.

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

* [PATCH 0/4] Add of_node_put to avoid memory leak
@ 2010-08-31 15:48 ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Grant Likely
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

These patches introduce calls to of_node_put after various functions that
call of_node_get.


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

* [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Timur Tabi
  Cc: kernel-janitors, Grant Likely, linuxppc-dev, linux-kernel,
	devicetree-discuss

Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node or of_find_node_by_type.

This patch also substantially reorganizes the error handling code in the
function, to that it is possible first to jump to code that frees qe_port
and then to jump to code that also puts np.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
index 3f4848e..38a5ef0 100644
--- a/drivers/serial/ucc_uart.c
+++ b/drivers/serial/ucc_uart.c
@@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	ret = of_address_to_resource(np, 0, &res);
 	if (ret) {
 		dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
-		kfree(qe_port);
-		return ret;
+		goto out_free;
 	}
 	if (!res.start) {
 		dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.mapbase = res.start;
 
@@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		iprop = of_get_property(np, "device-id", NULL);
 		if (!iprop) {
-			kfree(qe_port);
 			dev_err(&ofdev->dev, "UCC is unspecified in "
 				"device tree\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
 	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
 		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->ucc_num = *iprop - 1;
 
@@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "rx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	qe_port->us_info.rx_clock = qe_clock_source(sprop);
 	if ((qe_port->us_info.rx_clock < QE_BRG1) ||
 	    (qe_port->us_info.rx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 #ifdef LOOPBACK
@@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "tx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->us_info.tx_clock = qe_clock_source(sprop);
 #endif
 	if ((qe_port->us_info.tx_clock < QE_BRG1) ||
 	    (qe_port->us_info.tx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	/* Get the port number, numbered 0-3 */
 	iprop = of_get_property(np, "port-number", NULL);
 	if (!iprop) {
 		dev_err(&ofdev->dev, "missing port-number in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.line = *iprop;
 	if (qe_port->port.line >= UCC_MAX_UART) {
 		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
 			UCC_MAX_UART - 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	qe_port->port.irq = irq_of_parse_and_map(np, 0);
 	if (qe_port->port.irq == NO_IRQ) {
 		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
 		       qe_port->ucc_num + 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	/*
@@ -1373,8 +1372,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		np = of_find_node_by_type(NULL, "qe");
 		if (!np) {
 			dev_err(&ofdev->dev, "could not find 'qe' node\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
@@ -1382,8 +1381,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		dev_err(&ofdev->dev,
 		       "missing brg-frequency in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_np;
 	}
 
 	if (*iprop)
@@ -1398,16 +1397,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		if (!iprop) {
 			dev_err(&ofdev->dev,
 				"missing QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 		if (*iprop)
 			qe_port->port.uartclk = *iprop / 2;
 		else {
 			dev_err(&ofdev->dev,
 				"invalid QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 	}
 
@@ -1445,8 +1444,7 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (ret) {
 		dev_err(&ofdev->dev, "could not add /dev/ttyQE%u\n",
 		       qe_port->port.line);
-		kfree(qe_port);
-		return ret;
+		goto out_np;
 	}
 
 	dev_set_drvdata(&ofdev->dev, qe_port);
@@ -1460,6 +1458,11 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	       SERIAL_QE_MINOR + qe_port->port.line);
 
 	return 0;
+out_np:
+	of_node_put(np);
+out_free:
+	kfree(qe_port);
+	return ret;
 }
 
 static int ucc_uart_remove(struct platform_device *ofdev)


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

* [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Timur Tabi
  Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node or of_find_node_by_type.

This patch also substantially reorganizes the error handling code in the
function, to that it is possible first to jump to code that frees qe_port
and then to jump to code that also puts np.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

---
 drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
index 3f4848e..38a5ef0 100644
--- a/drivers/serial/ucc_uart.c
+++ b/drivers/serial/ucc_uart.c
@@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	ret = of_address_to_resource(np, 0, &res);
 	if (ret) {
 		dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
-		kfree(qe_port);
-		return ret;
+		goto out_free;
 	}
 	if (!res.start) {
 		dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.mapbase = res.start;
 
@@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		iprop = of_get_property(np, "device-id", NULL);
 		if (!iprop) {
-			kfree(qe_port);
 			dev_err(&ofdev->dev, "UCC is unspecified in "
 				"device tree\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
 	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
 		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->ucc_num = *iprop - 1;
 
@@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "rx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	qe_port->us_info.rx_clock = qe_clock_source(sprop);
 	if ((qe_port->us_info.rx_clock < QE_BRG1) ||
 	    (qe_port->us_info.rx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 #ifdef LOOPBACK
@@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "tx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->us_info.tx_clock = qe_clock_source(sprop);
 #endif
 	if ((qe_port->us_info.tx_clock < QE_BRG1) ||
 	    (qe_port->us_info.tx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	/* Get the port number, numbered 0-3 */
 	iprop = of_get_property(np, "port-number", NULL);
 	if (!iprop) {
 		dev_err(&ofdev->dev, "missing port-number in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.line = *iprop;
 	if (qe_port->port.line >= UCC_MAX_UART) {
 		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
 			UCC_MAX_UART - 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	qe_port->port.irq = irq_of_parse_and_map(np, 0);
 	if (qe_port->port.irq == NO_IRQ) {
 		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
 		       qe_port->ucc_num + 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	/*
@@ -1373,8 +1372,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		np = of_find_node_by_type(NULL, "qe");
 		if (!np) {
 			dev_err(&ofdev->dev, "could not find 'qe' node\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
@@ -1382,8 +1381,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		dev_err(&ofdev->dev,
 		       "missing brg-frequency in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_np;
 	}
 
 	if (*iprop)
@@ -1398,16 +1397,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		if (!iprop) {
 			dev_err(&ofdev->dev,
 				"missing QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 		if (*iprop)
 			qe_port->port.uartclk = *iprop / 2;
 		else {
 			dev_err(&ofdev->dev,
 				"invalid QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 	}
 
@@ -1445,8 +1444,7 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (ret) {
 		dev_err(&ofdev->dev, "could not add /dev/ttyQE%u\n",
 		       qe_port->port.line);
-		kfree(qe_port);
-		return ret;
+		goto out_np;
 	}
 
 	dev_set_drvdata(&ofdev->dev, qe_port);
@@ -1460,6 +1458,11 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	       SERIAL_QE_MINOR + qe_port->port.line);
 
 	return 0;
+out_np:
+	of_node_put(np);
+out_free:
+	kfree(qe_port);
+	return ret;
 }
 
 static int ucc_uart_remove(struct platform_device *ofdev)

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

* [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Timur Tabi
  Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node or of_find_node_by_type.

This patch also substantially reorganizes the error handling code in the
function, to that it is possible first to jump to code that frees qe_port
and then to jump to code that also puts np.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x = NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
index 3f4848e..38a5ef0 100644
--- a/drivers/serial/ucc_uart.c
+++ b/drivers/serial/ucc_uart.c
@@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	ret = of_address_to_resource(np, 0, &res);
 	if (ret) {
 		dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
-		kfree(qe_port);
-		return ret;
+		goto out_free;
 	}
 	if (!res.start) {
 		dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.mapbase = res.start;
 
@@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		iprop = of_get_property(np, "device-id", NULL);
 		if (!iprop) {
-			kfree(qe_port);
 			dev_err(&ofdev->dev, "UCC is unspecified in "
 				"device tree\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
 	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
 		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->ucc_num = *iprop - 1;
 
@@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "rx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	qe_port->us_info.rx_clock = qe_clock_source(sprop);
 	if ((qe_port->us_info.rx_clock < QE_BRG1) ||
 	    (qe_port->us_info.rx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 #ifdef LOOPBACK
@@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "tx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->us_info.tx_clock = qe_clock_source(sprop);
 #endif
 	if ((qe_port->us_info.tx_clock < QE_BRG1) ||
 	    (qe_port->us_info.tx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	/* Get the port number, numbered 0-3 */
 	iprop = of_get_property(np, "port-number", NULL);
 	if (!iprop) {
 		dev_err(&ofdev->dev, "missing port-number in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.line = *iprop;
 	if (qe_port->port.line >= UCC_MAX_UART) {
 		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
 			UCC_MAX_UART - 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	qe_port->port.irq = irq_of_parse_and_map(np, 0);
 	if (qe_port->port.irq = NO_IRQ) {
 		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
 		       qe_port->ucc_num + 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	/*
@@ -1373,8 +1372,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		np = of_find_node_by_type(NULL, "qe");
 		if (!np) {
 			dev_err(&ofdev->dev, "could not find 'qe' node\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
@@ -1382,8 +1381,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		dev_err(&ofdev->dev,
 		       "missing brg-frequency in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_np;
 	}
 
 	if (*iprop)
@@ -1398,16 +1397,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		if (!iprop) {
 			dev_err(&ofdev->dev,
 				"missing QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 		if (*iprop)
 			qe_port->port.uartclk = *iprop / 2;
 		else {
 			dev_err(&ofdev->dev,
 				"invalid QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 	}
 
@@ -1445,8 +1444,7 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (ret) {
 		dev_err(&ofdev->dev, "could not add /dev/ttyQE%u\n",
 		       qe_port->port.line);
-		kfree(qe_port);
-		return ret;
+		goto out_np;
 	}
 
 	dev_set_drvdata(&ofdev->dev, qe_port);
@@ -1460,6 +1458,11 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	       SERIAL_QE_MINOR + qe_port->port.line);
 
 	return 0;
+out_np:
+	of_node_put(np);
+out_free:
+	kfree(qe_port);
+	return ret;
 }
 
 static int ucc_uart_remove(struct platform_device *ofdev)


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

* [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Timur Tabi
  Cc: linuxppc-dev, devicetree-discuss, kernel-janitors, linux-kernel

Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node or of_find_node_by_type.

This patch also substantially reorganizes the error handling code in the
function, to that it is possible first to jump to code that frees qe_port
and then to jump to code that also puts np.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
index 3f4848e..38a5ef0 100644
--- a/drivers/serial/ucc_uart.c
+++ b/drivers/serial/ucc_uart.c
@@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	ret = of_address_to_resource(np, 0, &res);
 	if (ret) {
 		dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
-		kfree(qe_port);
-		return ret;
+		goto out_free;
 	}
 	if (!res.start) {
 		dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.mapbase = res.start;
 
@@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		iprop = of_get_property(np, "device-id", NULL);
 		if (!iprop) {
-			kfree(qe_port);
 			dev_err(&ofdev->dev, "UCC is unspecified in "
 				"device tree\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
 	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
 		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->ucc_num = *iprop - 1;
 
@@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "rx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	qe_port->us_info.rx_clock = qe_clock_source(sprop);
 	if ((qe_port->us_info.rx_clock < QE_BRG1) ||
 	    (qe_port->us_info.rx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 #ifdef LOOPBACK
@@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	sprop = of_get_property(np, "tx-clock-name", NULL);
 	if (!sprop) {
 		dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 	qe_port->us_info.tx_clock = qe_clock_source(sprop);
 #endif
 	if ((qe_port->us_info.tx_clock < QE_BRG1) ||
 	    (qe_port->us_info.tx_clock > QE_BRG16)) {
 		dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
-		kfree(qe_port);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_free;
 	}
 
 	/* Get the port number, numbered 0-3 */
 	iprop = of_get_property(np, "port-number", NULL);
 	if (!iprop) {
 		dev_err(&ofdev->dev, "missing port-number in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 	qe_port->port.line = *iprop;
 	if (qe_port->port.line >= UCC_MAX_UART) {
 		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
 			UCC_MAX_UART - 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	qe_port->port.irq = irq_of_parse_and_map(np, 0);
 	if (qe_port->port.irq == NO_IRQ) {
 		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
 		       qe_port->ucc_num + 1);
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_free;
 	}
 
 	/*
@@ -1373,8 +1372,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		np = of_find_node_by_type(NULL, "qe");
 		if (!np) {
 			dev_err(&ofdev->dev, "could not find 'qe' node\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_free;
 		}
 	}
 
@@ -1382,8 +1381,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (!iprop) {
 		dev_err(&ofdev->dev,
 		       "missing brg-frequency in device tree\n");
-		kfree(qe_port);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_np;
 	}
 
 	if (*iprop)
@@ -1398,16 +1397,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 		if (!iprop) {
 			dev_err(&ofdev->dev,
 				"missing QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 		if (*iprop)
 			qe_port->port.uartclk = *iprop / 2;
 		else {
 			dev_err(&ofdev->dev,
 				"invalid QE bus-frequency in device tree\n");
-			kfree(qe_port);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_np;
 		}
 	}
 
@@ -1445,8 +1444,7 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	if (ret) {
 		dev_err(&ofdev->dev, "could not add /dev/ttyQE%u\n",
 		       qe_port->port.line);
-		kfree(qe_port);
-		return ret;
+		goto out_np;
 	}
 
 	dev_set_drvdata(&ofdev->dev, qe_port);
@@ -1460,6 +1458,11 @@ static int ucc_uart_probe(struct platform_device *ofdev,
 	       SERIAL_QE_MINOR + qe_port->port.line);
 
 	return 0;
+out_np:
+	of_node_put(np);
+out_free:
+	kfree(qe_port);
+	return ret;
 }
 
 static int ucc_uart_remove(struct platform_device *ofdev)

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

* [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
  2010-08-31 15:48 ` Julia Lawall
  (?)
@ 2010-08-31 15:48   ` Julia Lawall
  -1 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Liam Girdwood
  Cc: kernel-janitors, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Grant Likely, alsa-devel, linux-kernel, devicetree-discuss

Add a call to of_node_put in the error handling code following a call to
of_parse_phandle.

This patch also moves the existing call to of_node_put tothe end of the
error handling code, to make it possible to jump to of_node_put without
doing the other cleanup operations.  These appear to be disjoint
operations, so the ordering doesn't matter.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 sound/soc/fsl/p1022_ds.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index f8176e8..63b9eaa 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -346,8 +346,10 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	}
 
 	mdata = kzalloc(sizeof(struct machine_data), GFP_KERNEL);
-	if (!mdata)
-		return -ENOMEM;
+	if (!mdata) {
+		ret = -ENOMEM;
+		goto error_put;
+	}
 
 	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
 	mdata->dai[0].ops = &p1022_ds_ops;
@@ -502,13 +504,12 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	return 0;
 
 error:
-	of_node_put(codec_np);
-
 	if (sound_device)
 		platform_device_unregister(sound_device);
 
 	kfree(mdata);
-
+error_put:
+	of_node_put(codec_np);
 	return ret;
 }
 


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

* [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Liam Girdwood
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, Mark Brown,
	kernel-janitors, linux-kernel, Grant Likely

Add a call to of_node_put in the error handling code following a call to
of_parse_phandle.

This patch also moves the existing call to of_node_put tothe end of the
error handling code, to make it possible to jump to of_node_put without
doing the other cleanup operations.  These appear to be disjoint
operations, so the ordering doesn't matter.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 sound/soc/fsl/p1022_ds.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index f8176e8..63b9eaa 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -346,8 +346,10 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	}
 
 	mdata = kzalloc(sizeof(struct machine_data), GFP_KERNEL);
-	if (!mdata)
-		return -ENOMEM;
+	if (!mdata) {
+		ret = -ENOMEM;
+		goto error_put;
+	}
 
 	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
 	mdata->dai[0].ops = &p1022_ds_ops;
@@ -502,13 +504,12 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	return 0;
 
 error:
-	of_node_put(codec_np);
-
 	if (sound_device)
 		platform_device_unregister(sound_device);
 
 	kfree(mdata);
-
+error_put:
+	of_node_put(codec_np);
 	return ret;
 }

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

* [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Liam Girdwood
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, Mark Brown,
	kernel-janitors, linux-kernel, Grant Likely

Add a call to of_node_put in the error handling code following a call to
of_parse_phandle.

This patch also moves the existing call to of_node_put tothe end of the
error handling code, to make it possible to jump to of_node_put without
doing the other cleanup operations.  These appear to be disjoint
operations, so the ordering doesn't matter.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x = NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 sound/soc/fsl/p1022_ds.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index f8176e8..63b9eaa 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -346,8 +346,10 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	}
 
 	mdata = kzalloc(sizeof(struct machine_data), GFP_KERNEL);
-	if (!mdata)
-		return -ENOMEM;
+	if (!mdata) {
+		ret = -ENOMEM;
+		goto error_put;
+	}
 
 	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
 	mdata->dai[0].ops = &p1022_ds_ops;
@@ -502,13 +504,12 @@ static int p1022_ds_probe(struct platform_device *pdev)
 	return 0;
 
 error:
-	of_node_put(codec_np);
-
 	if (sound_device)
 		platform_device_unregister(sound_device);
 
 	kfree(mdata);
-
+error_put:
+	of_node_put(codec_np);
 	return ret;
 }
 


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

* [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: kernel-janitors, Grant Likely, socketcan-core, netdev,
	linux-kernel, devicetree-discuss

Add a call to of_node_put in the error handling code following a call to
of_find_matching_node.

This patch also moves the existing call to of_node_put after the call to
iounmap in the error handling code, to make it possible to jump to
of_node_put without doing iounmap.  These appear to be disjoint operations,
so the ordering doesn't matter.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/can/mscan/mpc5xxx_can.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index b1bdc90..883959c 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -148,7 +148,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 	clockctl = of_iomap(np_clock, 0);
 	if (!clockctl) {
 		dev_err(&ofdev->dev, "couldn't map clock registers\n");
-		return 0;
+		goto exit_put;
 	}
 
 	/* Determine the MSCAN device index from the physical address */
@@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
 
 exit_unmap:
-	of_node_put(np_clock);
 	iounmap(clockctl);
-
+exit_put:
+	of_node_put(np_clock);
 	return freq;
 }
 #else /* !CONFIG_PPC_MPC512x */


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

* [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Add a call to of_node_put in the error handling code following a call to
of_find_matching_node.

This patch also moves the existing call to of_node_put after the call to
iounmap in the error handling code, to make it possible to jump to
of_node_put without doing iounmap.  These appear to be disjoint operations,
so the ordering doesn't matter.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

---
 drivers/net/can/mscan/mpc5xxx_can.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index b1bdc90..883959c 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -148,7 +148,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 	clockctl = of_iomap(np_clock, 0);
 	if (!clockctl) {
 		dev_err(&ofdev->dev, "couldn't map clock registers\n");
-		return 0;
+		goto exit_put;
 	}
 
 	/* Determine the MSCAN device index from the physical address */
@@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
 
 exit_unmap:
-	of_node_put(np_clock);
 	iounmap(clockctl);
-
+exit_put:
+	of_node_put(np_clock);
 	return freq;
 }
 #else /* !CONFIG_PPC_MPC512x */

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

* [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Add a call to of_node_put in the error handling code following a call to
of_find_matching_node.

This patch also moves the existing call to of_node_put after the call to
iounmap in the error handling code, to make it possible to jump to
of_node_put without doing iounmap.  These appear to be disjoint operations,
so the ordering doesn't matter.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x = NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/can/mscan/mpc5xxx_can.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index b1bdc90..883959c 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -148,7 +148,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 	clockctl = of_iomap(np_clock, 0);
 	if (!clockctl) {
 		dev_err(&ofdev->dev, "couldn't map clock registers\n");
-		return 0;
+		goto exit_put;
 	}
 
 	/* Determine the MSCAN device index from the physical address */
@@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 		clocksrc = 1 ? "ref_clk" : "sys_clk", clockdiv);
 
 exit_unmap:
-	of_node_put(np_clock);
 	iounmap(clockctl);
-
+exit_put:
+	of_node_put(np_clock);
 	return freq;
 }
 #else /* !CONFIG_PPC_MPC512x */


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

* [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
  2010-08-31 15:48 ` Julia Lawall
  (?)
@ 2010-08-31 15:48   ` Julia Lawall
  -1 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kernel-janitors, Paul Mackerras, Grant Likely, linuxppc-dev,
	linux-kernel, devicetree-discuss

Add a call to of_node_put in the error handling code following a call to
of_find_node_by_type.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 arch/powerpc/platforms/chrp/nvram.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/chrp/nvram.c b/arch/powerpc/platforms/chrp/nvram.c
index ba3588f..d3ceff0 100644
--- a/arch/powerpc/platforms/chrp/nvram.c
+++ b/arch/powerpc/platforms/chrp/nvram.c
@@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
 		return;
 
 	nbytes_p = of_get_property(nvram, "#bytes", &proplen);
-	if (nbytes_p == NULL || proplen != sizeof(unsigned int))
+	if (nbytes_p == NULL || proplen != sizeof(unsigned int)) {
+		of_node_put(nvram);
 		return;
+	}
 
 	nvram_size = *nbytes_p;
 


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

* [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kernel-janitors, Paul Mackerras, Grant Likely, linuxppc-dev,
	linux-kernel, devicetree-discuss

Add a call to of_node_put in the error handling code following a call to
of_find_node_by_type.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x = NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 arch/powerpc/platforms/chrp/nvram.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/chrp/nvram.c b/arch/powerpc/platforms/chrp/nvram.c
index ba3588f..d3ceff0 100644
--- a/arch/powerpc/platforms/chrp/nvram.c
+++ b/arch/powerpc/platforms/chrp/nvram.c
@@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
 		return;
 
 	nbytes_p = of_get_property(nvram, "#bytes", &proplen);
-	if (nbytes_p = NULL || proplen != sizeof(unsigned int))
+	if (nbytes_p = NULL || proplen != sizeof(unsigned int)) {
+		of_node_put(nvram);
 		return;
+	}
 
 	nvram_size = *nbytes_p;
 


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

* [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:48   ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 15:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: devicetree-discuss, kernel-janitors, linux-kernel,
	Paul Mackerras, linuxppc-dev

Add a call to of_node_put in the error handling code following a call to
of_find_node_by_type.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 arch/powerpc/platforms/chrp/nvram.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/chrp/nvram.c b/arch/powerpc/platforms/chrp/nvram.c
index ba3588f..d3ceff0 100644
--- a/arch/powerpc/platforms/chrp/nvram.c
+++ b/arch/powerpc/platforms/chrp/nvram.c
@@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
 		return;
 
 	nbytes_p = of_get_property(nvram, "#bytes", &proplen);
-	if (nbytes_p == NULL || proplen != sizeof(unsigned int))
+	if (nbytes_p == NULL || proplen != sizeof(unsigned int)) {
+		of_node_put(nvram);
 		return;
+	}
 
 	nvram_size = *nbytes_p;
 

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

* Re: [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
  2010-08-31 15:48   ` Julia Lawall
  (?)
@ 2010-08-31 15:59     ` Mark Brown
  -1 siblings, 0 replies; 59+ messages in thread
From: Mark Brown @ 2010-08-31 15:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Liam Girdwood, kernel-janitors, Jaroslav Kysela, Takashi Iwai,
	Grant Likely, alsa-devel, linux-kernel, devicetree-discuss,
	Timur Tabi

On Tue, Aug 31, 2010 at 05:48:56PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.
> 
> This patch also moves the existing call to of_node_put tothe end of the
> error handling code, to make it possible to jump to of_node_put without
> doing the other cleanup operations.  These appear to be disjoint
> operations, so the ordering doesn't matter.

CCing in Timur who wrote and maintains that driver - looks OK to me.

> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  sound/soc/fsl/p1022_ds.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
> index f8176e8..63b9eaa 100644
> --- a/sound/soc/fsl/p1022_ds.c
> +++ b/sound/soc/fsl/p1022_ds.c
> @@ -346,8 +346,10 @@ static int p1022_ds_probe(struct platform_device *pdev)
>  	}
>  
>  	mdata = kzalloc(sizeof(struct machine_data), GFP_KERNEL);
> -	if (!mdata)
> -		return -ENOMEM;
> +	if (!mdata) {
> +		ret = -ENOMEM;
> +		goto error_put;
> +	}
>  
>  	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
>  	mdata->dai[0].ops = &p1022_ds_ops;
> @@ -502,13 +504,12 @@ static int p1022_ds_probe(struct platform_device *pdev)
>  	return 0;
>  
>  error:
> -	of_node_put(codec_np);
> -
>  	if (sound_device)
>  		platform_device_unregister(sound_device);
>  
>  	kfree(mdata);
> -
> +error_put:
> +	of_node_put(codec_np);
>  	return ret;
>  }
>  
> 

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

* Re: [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
@ 2010-08-31 15:59     ` Mark Brown
  0 siblings, 0 replies; 59+ messages in thread
From: Mark Brown @ 2010-08-31 15:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, kernel-janitors,
	linux-kernel, Grant Likely, Timur Tabi, Liam Girdwood

On Tue, Aug 31, 2010 at 05:48:56PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.
> 
> This patch also moves the existing call to of_node_put tothe end of the
> error handling code, to make it possible to jump to of_node_put without
> doing the other cleanup operations.  These appear to be disjoint
> operations, so the ordering doesn't matter.

CCing in Timur who wrote and maintains that driver - looks OK to me.

> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  sound/soc/fsl/p1022_ds.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
> index f8176e8..63b9eaa 100644
> --- a/sound/soc/fsl/p1022_ds.c
> +++ b/sound/soc/fsl/p1022_ds.c
> @@ -346,8 +346,10 @@ static int p1022_ds_probe(struct platform_device *pdev)
>  	}
>  
>  	mdata = kzalloc(sizeof(struct machine_data), GFP_KERNEL);
> -	if (!mdata)
> -		return -ENOMEM;
> +	if (!mdata) {
> +		ret = -ENOMEM;
> +		goto error_put;
> +	}
>  
>  	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
>  	mdata->dai[0].ops = &p1022_ds_ops;
> @@ -502,13 +504,12 @@ static int p1022_ds_probe(struct platform_device *pdev)
>  	return 0;
>  
>  error:
> -	of_node_put(codec_np);
> -
>  	if (sound_device)
>  		platform_device_unregister(sound_device);
>  
>  	kfree(mdata);
> -
> +error_put:
> +	of_node_put(codec_np);
>  	return ret;
>  }
>  
> 

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

* Re: [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid
@ 2010-08-31 15:59     ` Mark Brown
  0 siblings, 0 replies; 59+ messages in thread
From: Mark Brown @ 2010-08-31 15:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, kernel-janitors,
	linux-kernel, Grant Likely, Timur Tabi, Liam Girdwood

On Tue, Aug 31, 2010 at 05:48:56PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.
> 
> This patch also moves the existing call to of_node_put tothe end of the
> error handling code, to make it possible to jump to of_node_put without
> doing the other cleanup operations.  These appear to be disjoint
> operations, so the ordering doesn't matter.

CCing in Timur who wrote and maintains that driver - looks OK to me.

> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x = NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  sound/soc/fsl/p1022_ds.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
> index f8176e8..63b9eaa 100644
> --- a/sound/soc/fsl/p1022_ds.c
> +++ b/sound/soc/fsl/p1022_ds.c
> @@ -346,8 +346,10 @@ static int p1022_ds_probe(struct platform_device *pdev)
>  	}
>  
>  	mdata = kzalloc(sizeof(struct machine_data), GFP_KERNEL);
> -	if (!mdata)
> -		return -ENOMEM;
> +	if (!mdata) {
> +		ret = -ENOMEM;
> +		goto error_put;
> +	}
>  
>  	mdata->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
>  	mdata->dai[0].ops = &p1022_ds_ops;
> @@ -502,13 +504,12 @@ static int p1022_ds_probe(struct platform_device *pdev)
>  	return 0;
>  
>  error:
> -	of_node_put(codec_np);
> -
>  	if (sound_device)
>  		platform_device_unregister(sound_device);
>  
>  	kfree(mdata);
> -
> +error_put:
> +	of_node_put(codec_np);
>  	return ret;
>  }
>  
> 

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 16:51     ` Wolfram Sang
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfram Sang @ 2010-08-31 16:51 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Wolfgang Grandegger, kernel-janitors, Grant Likely,
	socketcan-core, netdev, linux-kernel, devicetree-discuss

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

On Tue, Aug 31, 2010 at 05:48:57PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_matching_node.
> 
> This patch also moves the existing call to of_node_put after the call to
> iounmap in the error handling code, to make it possible to jump to
> of_node_put without doing iounmap.  These appear to be disjoint operations,
> so the ordering doesn't matter.

Hmm, the function you change returns -ENODEV if a clock cannot be found, but
the return type is u32. This should better be 0. Can you take care of this as
well? You could then add my

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/net/can/mscan/mpc5xxx_can.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> index b1bdc90..883959c 100644
> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> @@ -148,7 +148,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  	clockctl = of_iomap(np_clock, 0);
>  	if (!clockctl) {
>  		dev_err(&ofdev->dev, "couldn't map clock registers\n");
> -		return 0;
> +		goto exit_put;
>  	}
>  
>  	/* Determine the MSCAN device index from the physical address */
> @@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
>  
>  exit_unmap:
> -	of_node_put(np_clock);
>  	iounmap(clockctl);
> -
> +exit_put:
> +	of_node_put(np_clock);
>  	return freq;
>  }
>  #else /* !CONFIG_PPC_MPC512x */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 16:51     ` Wolfram Sang
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfram Sang @ 2010-08-31 16:51 UTC (permalink / raw)
  To: Julia Lawall
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA


[-- Attachment #1.1: Type: text/plain, Size: 3014 bytes --]

On Tue, Aug 31, 2010 at 05:48:57PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_matching_node.
> 
> This patch also moves the existing call to of_node_put after the call to
> iounmap in the error handling code, to make it possible to jump to
> of_node_put without doing iounmap.  These appear to be disjoint operations,
> so the ordering doesn't matter.

Hmm, the function you change returns -ENODEV if a clock cannot be found, but
the return type is u32. This should better be 0. Can you take care of this as
well? You could then add my

Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> 
> ---
>  drivers/net/can/mscan/mpc5xxx_can.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> index b1bdc90..883959c 100644
> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> @@ -148,7 +148,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  	clockctl = of_iomap(np_clock, 0);
>  	if (!clockctl) {
>  		dev_err(&ofdev->dev, "couldn't map clock registers\n");
> -		return 0;
> +		goto exit_put;
>  	}
>  
>  	/* Determine the MSCAN device index from the physical address */
> @@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
>  
>  exit_unmap:
> -	of_node_put(np_clock);
>  	iounmap(clockctl);
> -
> +exit_put:
> +	of_node_put(np_clock);
>  	return freq;
>  }
>  #else /* !CONFIG_PPC_MPC512x */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add
@ 2010-08-31 16:51     ` Wolfram Sang
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfram Sang @ 2010-08-31 16:51 UTC (permalink / raw)
  To: Julia Lawall
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

On Tue, Aug 31, 2010 at 05:48:57PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_matching_node.
> 
> This patch also moves the existing call to of_node_put after the call to
> iounmap in the error handling code, to make it possible to jump to
> of_node_put without doing iounmap.  These appear to be disjoint operations,
> so the ordering doesn't matter.

Hmm, the function you change returns -ENODEV if a clock cannot be found, but
the return type is u32. This should better be 0. Can you take care of this as
well? You could then add my

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/net/can/mscan/mpc5xxx_can.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> index b1bdc90..883959c 100644
> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> @@ -148,7 +148,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  	clockctl = of_iomap(np_clock, 0);
>  	if (!clockctl) {
>  		dev_err(&ofdev->dev, "couldn't map clock registers\n");
> -		return 0;
> +		goto exit_put;
>  	}
>  
>  	/* Determine the MSCAN device index from the physical address */
> @@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
>  
>  exit_unmap:
> -	of_node_put(np_clock);
>  	iounmap(clockctl);
> -
> +exit_put:
> +	of_node_put(np_clock);
>  	return freq;
>  }
>  #else /* !CONFIG_PPC_MPC512x */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
  2010-08-31 16:51     ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak Wolfram Sang
@ 2010-08-31 16:55       ` Julia Lawall
  -1 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 16:55 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Wolfgang Grandegger, kernel-janitors, Grant Likely,
	socketcan-core, netdev, linux-kernel, devicetree-discuss

On Tue, 31 Aug 2010, Wolfram Sang wrote:

> On Tue, Aug 31, 2010 at 05:48:57PM +0200, Julia Lawall wrote:
> > Add a call to of_node_put in the error handling code following a call to
> > of_find_matching_node.
> > 
> > This patch also moves the existing call to of_node_put after the call to
> > iounmap in the error handling code, to make it possible to jump to
> > of_node_put without doing iounmap.  These appear to be disjoint operations,
> > so the ordering doesn't matter.
> 
> Hmm, the function you change returns -ENODEV if a clock cannot be found, but
> the return type is u32. This should better be 0. Can you take care of this as
> well?

I'll look at it.  I was trying to make the returned value be signed, which 
seemed to have some slightly complicated repercussions, but a return value 
of 0 may work out much better.

julia

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put
@ 2010-08-31 16:55       ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 16:55 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Wolfgang Grandegger, kernel-janitors, Grant Likely,
	socketcan-core, netdev, linux-kernel, devicetree-discuss

On Tue, 31 Aug 2010, Wolfram Sang wrote:

> On Tue, Aug 31, 2010 at 05:48:57PM +0200, Julia Lawall wrote:
> > Add a call to of_node_put in the error handling code following a call to
> > of_find_matching_node.
> > 
> > This patch also moves the existing call to of_node_put after the call to
> > iounmap in the error handling code, to make it possible to jump to
> > of_node_put without doing iounmap.  These appear to be disjoint operations,
> > so the ordering doesn't matter.
> 
> Hmm, the function you change returns -ENODEV if a clock cannot be found, but
> the return type is u32. This should better be 0. Can you take care of this as
> well?

I'll look at it.  I was trying to make the returned value be signed, which 
seemed to have some slightly complicated repercussions, but a return value 
of 0 may work out much better.

julia

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 17:44       ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 17:44 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Wolfgang Grandegger, kernel-janitors, Grant Likely,
	socketcan-core, netdev, linux-kernel, devicetree-discuss

Add a call to of_node_put in the error handling code following a call to
of_find_matching_node.

This patch also moves the existing call to of_node_put after the call to
iounmap in the error handling code, to make it possible to jump to
of_node_put without doing iounmap.  These appear to be disjoint operations,
so the ordering doesn't matter.

This patch furthermore changes the -ENODEV result in the error handling
code for of_find_matching_node to a return of 0, as found in the error
handling code for of_iomap, because the return type of the function is
unsigned.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/can/mscan/mpc5xxx_can.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -143,12 +143,12 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
 	if (!np_clock) {
 		dev_err(&ofdev->dev, "couldn't find clock node\n");
-		return -ENODEV;
+		return 0;
 	}
 	clockctl = of_iomap(np_clock, 0);
 	if (!clockctl) {
 		dev_err(&ofdev->dev, "couldn't map clock registers\n");
-		return 0;
+		goto exit_put;
 	}
 
 	/* Determine the MSCAN device index from the physical address */
@@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
 
 exit_unmap:
-	of_node_put(np_clock);
 	iounmap(clockctl);
-
+exit_put:
+	of_node_put(np_clock);
 	return freq;
 }
 #else /* !CONFIG_PPC_MPC512x */

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 17:44       ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 17:44 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Add a call to of_node_put in the error handling code following a call to
of_find_matching_node.

This patch also moves the existing call to of_node_put after the call to
iounmap in the error handling code, to make it possible to jump to
of_node_put without doing iounmap.  These appear to be disjoint operations,
so the ordering doesn't matter.

This patch furthermore changes the -ENODEV result in the error handling
code for of_find_matching_node to a return of 0, as found in the error
handling code for of_iomap, because the return type of the function is
unsigned.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

---
 drivers/net/can/mscan/mpc5xxx_can.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -143,12 +143,12 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
 	if (!np_clock) {
 		dev_err(&ofdev->dev, "couldn't find clock node\n");
-		return -ENODEV;
+		return 0;
 	}
 	clockctl = of_iomap(np_clock, 0);
 	if (!clockctl) {
 		dev_err(&ofdev->dev, "couldn't map clock registers\n");
-		return 0;
+		goto exit_put;
 	}
 
 	/* Determine the MSCAN device index from the physical address */
@@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
 
 exit_unmap:
-	of_node_put(np_clock);
 	iounmap(clockctl);
-
+exit_put:
+	of_node_put(np_clock);
 	return freq;
 }
 #else /* !CONFIG_PPC_MPC512x */

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put
@ 2010-08-31 17:44       ` Julia Lawall
  0 siblings, 0 replies; 59+ messages in thread
From: Julia Lawall @ 2010-08-31 17:44 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Add a call to of_node_put in the error handling code following a call to
of_find_matching_node.

This patch also moves the existing call to of_node_put after the call to
iounmap in the error handling code, to make it possible to jump to
of_node_put without doing iounmap.  These appear to be disjoint operations,
so the ordering doesn't matter.

This patch furthermore changes the -ENODEV result in the error handling
code for of_find_matching_node to a return of 0, as found in the error
handling code for of_iomap, because the return type of the function is
unsigned.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@

*x = 
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x = NULL) S
<... when != x = E
*if (...) {
  ... when != of_node_put(x)
      when != if (...) { ... of_node_put(x); ... }
(
  return <+...x...+>;
|
*  return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/can/mscan/mpc5xxx_can.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -143,12 +143,12 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
 	if (!np_clock) {
 		dev_err(&ofdev->dev, "couldn't find clock node\n");
-		return -ENODEV;
+		return 0;
 	}
 	clockctl = of_iomap(np_clock, 0);
 	if (!clockctl) {
 		dev_err(&ofdev->dev, "couldn't map clock registers\n");
-		return 0;
+		goto exit_put;
 	}
 
 	/* Determine the MSCAN device index from the physical address */
@@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 		clocksrc = 1 ? "ref_clk" : "sys_clk", clockdiv);
 
 exit_unmap:
-	of_node_put(np_clock);
 	iounmap(clockctl);
-
+exit_put:
+	of_node_put(np_clock);
 	return freq;
 }
 #else /* !CONFIG_PPC_MPC512x */

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 19:10         ` Wolfram Sang
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfram Sang @ 2010-08-31 19:10 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Wolfgang Grandegger, kernel-janitors, Grant Likely,
	socketcan-core, netdev, linux-kernel, devicetree-discuss

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

On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_matching_node.
> 
> This patch also moves the existing call to of_node_put after the call to
> iounmap in the error handling code, to make it possible to jump to
> of_node_put without doing iounmap.  These appear to be disjoint operations,
> so the ordering doesn't matter.
> 
> This patch furthermore changes the -ENODEV result in the error handling
> code for of_find_matching_node to a return of 0, as found in the error
> handling code for of_iomap, because the return type of the function is
> unsigned.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

> 
> ---
>  drivers/net/can/mscan/mpc5xxx_can.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> @@ -143,12 +143,12 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
>  	if (!np_clock) {
>  		dev_err(&ofdev->dev, "couldn't find clock node\n");
> -		return -ENODEV;
> +		return 0;
>  	}
>  	clockctl = of_iomap(np_clock, 0);
>  	if (!clockctl) {
>  		dev_err(&ofdev->dev, "couldn't map clock registers\n");
> -		return 0;
> +		goto exit_put;
>  	}
>  
>  	/* Determine the MSCAN device index from the physical address */
> @@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
>  
>  exit_unmap:
> -	of_node_put(np_clock);
>  	iounmap(clockctl);
> -
> +exit_put:
> +	of_node_put(np_clock);
>  	return freq;
>  }
>  #else /* !CONFIG_PPC_MPC512x */

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 19:10         ` Wolfram Sang
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfram Sang @ 2010-08-31 19:10 UTC (permalink / raw)
  To: Julia Lawall
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Likely,
	Wolfgang Grandegger


[-- Attachment #1.1: Type: text/plain, Size: 2938 bytes --]

On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_matching_node.
> 
> This patch also moves the existing call to of_node_put after the call to
> iounmap in the error handling code, to make it possible to jump to
> of_node_put without doing iounmap.  These appear to be disjoint operations,
> so the ordering doesn't matter.
> 
> This patch furthermore changes the -ENODEV result in the error handling
> code for of_find_matching_node to a return of 0, as found in the error
> handling code for of_iomap, because the return type of the function is
> unsigned.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

> 
> ---
>  drivers/net/can/mscan/mpc5xxx_can.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> @@ -143,12 +143,12 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
>  	if (!np_clock) {
>  		dev_err(&ofdev->dev, "couldn't find clock node\n");
> -		return -ENODEV;
> +		return 0;
>  	}
>  	clockctl = of_iomap(np_clock, 0);
>  	if (!clockctl) {
>  		dev_err(&ofdev->dev, "couldn't map clock registers\n");
> -		return 0;
> +		goto exit_put;
>  	}
>  
>  	/* Determine the MSCAN device index from the physical address */
> @@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
>  
>  exit_unmap:
> -	of_node_put(np_clock);
>  	iounmap(clockctl);
> -
> +exit_put:
> +	of_node_put(np_clock);
>  	return freq;
>  }
>  #else /* !CONFIG_PPC_MPC512x */

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add
@ 2010-08-31 19:10         ` Wolfram Sang
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfram Sang @ 2010-08-31 19:10 UTC (permalink / raw)
  To: Julia Lawall
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Likely,
	Wolfgang Grandegger

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

On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_matching_node.
> 
> This patch also moves the existing call to of_node_put after the call to
> iounmap in the error handling code, to make it possible to jump to
> of_node_put without doing iounmap.  These appear to be disjoint operations,
> so the ordering doesn't matter.
> 
> This patch furthermore changes the -ENODEV result in the error handling
> code for of_find_matching_node to a return of 0, as found in the error
> handling code for of_iomap, because the return type of the function is
> unsigned.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

> 
> ---
>  drivers/net/can/mscan/mpc5xxx_can.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> @@ -143,12 +143,12 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
>  	if (!np_clock) {
>  		dev_err(&ofdev->dev, "couldn't find clock node\n");
> -		return -ENODEV;
> +		return 0;
>  	}
>  	clockctl = of_iomap(np_clock, 0);
>  	if (!clockctl) {
>  		dev_err(&ofdev->dev, "couldn't map clock registers\n");
> -		return 0;
> +		goto exit_put;
>  	}
>  
>  	/* Determine the MSCAN device index from the physical address */
> @@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
>  		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
>  
>  exit_unmap:
> -	of_node_put(np_clock);
>  	iounmap(clockctl);
> -
> +exit_put:
> +	of_node_put(np_clock);
>  	return freq;
>  }
>  #else /* !CONFIG_PPC_MPC512x */

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 19:22           ` Wolfgang Grandegger
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfgang Grandegger @ 2010-08-31 19:22 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Julia Lawall, kernel-janitors, Grant Likely, socketcan-core,
	netdev, linux-kernel, devicetree-discuss

On 08/31/2010 09:10 PM, Wolfram Sang wrote:
> On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
>> Add a call to of_node_put in the error handling code following a call to
>> of_find_matching_node.
>>
>> This patch also moves the existing call to of_node_put after the call to
>> iounmap in the error handling code, to make it possible to jump to
>> of_node_put without doing iounmap.  These appear to be disjoint operations,
>> so the ordering doesn't matter.
>>
>> This patch furthermore changes the -ENODEV result in the error handling
>> code for of_find_matching_node to a return of 0, as found in the error
>> handling code for of_iomap, because the return type of the function is
>> unsigned.
>>
>> The semantic match that finds this problem is as follows:
>> (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @r exists@
>> local idexpression x;
>> expression E,E1,E2;
>> statement S;
>> @@
>>
>> *x = 
>> (of_find_node_by_path
>> |of_find_node_by_name
>> |of_find_node_by_phandle
>> |of_get_parent
>> |of_get_next_parent
>> |of_get_next_child
>> |of_find_compatible_node
>> |of_match_node
>> |of_find_node_by_type
>> |of_find_node_with_property
>> |of_find_matching_node
>> |of_parse_phandle
>> )(...);
>> ...
>> if (x == NULL) S
>> <... when != x = E
>> *if (...) {
>>   ... when != of_node_put(x)
>>       when != if (...) { ... of_node_put(x); ... }
>> (
>>   return <+...x...+>;
>> |
>> *  return ...;
>> )
>> }
>> ...>
>> (
>> E2 = x;
>> |
>> of_node_put(x);
>> )
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

Acked-by: Wolfgang Grandegger <wg@grandegger.com>

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-08-31 19:22           ` Wolfgang Grandegger
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfgang Grandegger @ 2010-08-31 19:22 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Likely, Julia Lawall

On 08/31/2010 09:10 PM, Wolfram Sang wrote:
> On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
>> Add a call to of_node_put in the error handling code following a call to
>> of_find_matching_node.
>>
>> This patch also moves the existing call to of_node_put after the call to
>> iounmap in the error handling code, to make it possible to jump to
>> of_node_put without doing iounmap.  These appear to be disjoint operations,
>> so the ordering doesn't matter.
>>
>> This patch furthermore changes the -ENODEV result in the error handling
>> code for of_find_matching_node to a return of 0, as found in the error
>> handling code for of_iomap, because the return type of the function is
>> unsigned.
>>
>> The semantic match that finds this problem is as follows:
>> (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @r exists@
>> local idexpression x;
>> expression E,E1,E2;
>> statement S;
>> @@
>>
>> *x = 
>> (of_find_node_by_path
>> |of_find_node_by_name
>> |of_find_node_by_phandle
>> |of_get_parent
>> |of_get_next_parent
>> |of_get_next_child
>> |of_find_compatible_node
>> |of_match_node
>> |of_find_node_by_type
>> |of_find_node_with_property
>> |of_find_matching_node
>> |of_parse_phandle
>> )(...);
>> ...
>> if (x == NULL) S
>> <... when != x = E
>> *if (...) {
>>   ... when != of_node_put(x)
>>       when != if (...) { ... of_node_put(x); ... }
>> (
>>   return <+...x...+>;
>> |
>> *  return ...;
>> )
>> }
>> ...>
>> (
>> E2 = x;
>> |
>> of_node_put(x);
>> )
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> 
> Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put
@ 2010-08-31 19:22           ` Wolfgang Grandegger
  0 siblings, 0 replies; 59+ messages in thread
From: Wolfgang Grandegger @ 2010-08-31 19:22 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Grant Likely, Julia Lawall

On 08/31/2010 09:10 PM, Wolfram Sang wrote:
> On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
>> Add a call to of_node_put in the error handling code following a call to
>> of_find_matching_node.
>>
>> This patch also moves the existing call to of_node_put after the call to
>> iounmap in the error handling code, to make it possible to jump to
>> of_node_put without doing iounmap.  These appear to be disjoint operations,
>> so the ordering doesn't matter.
>>
>> This patch furthermore changes the -ENODEV result in the error handling
>> code for of_find_matching_node to a return of 0, as found in the error
>> handling code for of_iomap, because the return type of the function is
>> unsigned.
>>
>> The semantic match that finds this problem is as follows:
>> (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @r exists@
>> local idexpression x;
>> expression E,E1,E2;
>> statement S;
>> @@
>>
>> *x = 
>> (of_find_node_by_path
>> |of_find_node_by_name
>> |of_find_node_by_phandle
>> |of_get_parent
>> |of_get_next_parent
>> |of_get_next_child
>> |of_find_compatible_node
>> |of_match_node
>> |of_find_node_by_type
>> |of_find_node_with_property
>> |of_find_matching_node
>> |of_parse_phandle
>> )(...);
>> ...
>> if (x = NULL) S
>> <... when != x = E
>> *if (...) {
>>   ... when != of_node_put(x)
>>       when != if (...) { ... of_node_put(x); ... }
>> (
>>   return <+...x...+>;
>> |
>> *  return ...;
>> )
>> }
>> ...>
>> (
>> E2 = x;
>> |
>> of_node_put(x);
>> )
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

Acked-by: Wolfgang Grandegger <wg@grandegger.com>

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-09-01 17:59             ` David Miller
  0 siblings, 0 replies; 59+ messages in thread
From: David Miller @ 2010-09-01 17:59 UTC (permalink / raw)
  To: wg
  Cc: w.sang, julia, kernel-janitors, grant.likely, socketcan-core,
	netdev, linux-kernel, devicetree-discuss

From: Wolfgang Grandegger <wg@grandegger.com>
Date: Tue, 31 Aug 2010 21:22:07 +0200

> On 08/31/2010 09:10 PM, Wolfram Sang wrote:
>> On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
>>> Add a call to of_node_put in the error handling code following a call to
>>> of_find_matching_node.
>>>
>>> This patch also moves the existing call to of_node_put after the call to
>>> iounmap in the error handling code, to make it possible to jump to
>>> of_node_put without doing iounmap.  These appear to be disjoint operations,
>>> so the ordering doesn't matter.
>>>
>>> This patch furthermore changes the -ENODEV result in the error handling
>>> code for of_find_matching_node to a return of 0, as found in the error
>>> handling code for of_iomap, because the return type of the function is
>>> unsigned.
>>>
>>> The semantic match that finds this problem is as follows:
>>> (http://coccinelle.lip6.fr/)
 ...
>>> Signed-off-by: Julia Lawall <julia@diku.dk>
>> 
>> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
> 
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>

Applied.

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
@ 2010-09-01 17:59             ` David Miller
  0 siblings, 0 replies; 59+ messages in thread
From: David Miller @ 2010-09-01 17:59 UTC (permalink / raw)
  To: wg-5Yr1BZd7O62+XT7JhA+gdA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	grant.likely-s3s/WqlpOiPyB63q8FvJNQ, julia-dAYI7NvHqcQ

From: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Date: Tue, 31 Aug 2010 21:22:07 +0200

> On 08/31/2010 09:10 PM, Wolfram Sang wrote:
>> On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
>>> Add a call to of_node_put in the error handling code following a call to
>>> of_find_matching_node.
>>>
>>> This patch also moves the existing call to of_node_put after the call to
>>> iounmap in the error handling code, to make it possible to jump to
>>> of_node_put without doing iounmap.  These appear to be disjoint operations,
>>> so the ordering doesn't matter.
>>>
>>> This patch furthermore changes the -ENODEV result in the error handling
>>> code for of_find_matching_node to a return of 0, as found in the error
>>> handling code for of_iomap, because the return type of the function is
>>> unsigned.
>>>
>>> The semantic match that finds this problem is as follows:
>>> (http://coccinelle.lip6.fr/)
 ...
>>> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
>> 
>> Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Applied.

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

* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add
@ 2010-09-01 17:59             ` David Miller
  0 siblings, 0 replies; 59+ messages in thread
From: David Miller @ 2010-09-01 17:59 UTC (permalink / raw)
  To: wg-5Yr1BZd7O62+XT7JhA+gdA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	grant.likely-s3s/WqlpOiPyB63q8FvJNQ, julia-dAYI7NvHqcQ

From: Wolfgang Grandegger <wg@grandegger.com>
Date: Tue, 31 Aug 2010 21:22:07 +0200

> On 08/31/2010 09:10 PM, Wolfram Sang wrote:
>> On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote:
>>> Add a call to of_node_put in the error handling code following a call to
>>> of_find_matching_node.
>>>
>>> This patch also moves the existing call to of_node_put after the call to
>>> iounmap in the error handling code, to make it possible to jump to
>>> of_node_put without doing iounmap.  These appear to be disjoint operations,
>>> so the ordering doesn't matter.
>>>
>>> This patch furthermore changes the -ENODEV result in the error handling
>>> code for of_find_matching_node to a return of 0, as found in the error
>>> handling code for of_iomap, because the return type of the function is
>>> unsigned.
>>>
>>> The semantic match that finds this problem is as follows:
>>> (http://coccinelle.lip6.fr/)
 ...
>>> Signed-off-by: Julia Lawall <julia@diku.dk>
>> 
>> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
> 
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>

Applied.

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

* Re: [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
  2010-08-31 15:48   ` Julia Lawall
@ 2010-09-01 18:39     ` Timur Tabi
  -1 siblings, 0 replies; 59+ messages in thread
From: Timur Tabi @ 2010-09-01 18:39 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Liam Girdwood, alsa-devel, Takashi Iwai, devicetree-discuss,
	Mark Brown, kernel-janitors, linux-kernel, Grant Likely

On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.
>
> This patch also moves the existing call to of_node_put tothe end of the
> error handling code, to make it possible to jump to of_node_put without
> doing the other cleanup operations.  These appear to be disjoint
> operations, so the ordering doesn't matter.

Acked-by: Timur Tabi <timur@freescale.com>

Thanks, Julia.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add
@ 2010-09-01 18:39     ` Timur Tabi
  0 siblings, 0 replies; 59+ messages in thread
From: Timur Tabi @ 2010-09-01 18:39 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Liam Girdwood, alsa-devel, Takashi Iwai, devicetree-discuss,
	Mark Brown, kernel-janitors, linux-kernel, Grant Likely

On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.
>
> This patch also moves the existing call to of_node_put tothe end of the
> error handling code, to make it possible to jump to of_node_put without
> doing the other cleanup operations.  These appear to be disjoint
> operations, so the ordering doesn't matter.

Acked-by: Timur Tabi <timur@freescale.com>

Thanks, Julia.

-- 
Timur Tabi
Linux kernel developer at Freescale
--
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] 59+ messages in thread

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-09-01 18:40     ` Timur Tabi
  0 siblings, 0 replies; 59+ messages in thread
From: Timur Tabi @ 2010-09-01 18:40 UTC (permalink / raw)
  To: Julia Lawall
  Cc: linuxppc-dev, devicetree-discuss, kernel-janitors, linux-kernel

On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
>
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.

Acked-by: Timur Tabi <timur@freescale.com>

Thanks, Julia.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-09-01 18:40     ` Timur Tabi
  0 siblings, 0 replies; 59+ messages in thread
From: Timur Tabi @ 2010-09-01 18:40 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org> wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
>
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.

Acked-by: Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Thanks, Julia.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid
@ 2010-09-01 18:40     ` Timur Tabi
  0 siblings, 0 replies; 59+ messages in thread
From: Timur Tabi @ 2010-09-01 18:40 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
>
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.

Acked-by: Timur Tabi <timur@freescale.com>

Thanks, Julia.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-09-01 18:40     ` Timur Tabi
  0 siblings, 0 replies; 59+ messages in thread
From: Timur Tabi @ 2010-09-01 18:40 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, devicetree-discuss, linuxppc-dev, linux-kernel

On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
>
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.

Acked-by: Timur Tabi <timur@freescale.com>

Thanks, Julia.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

* Re: [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
  2010-09-01 18:39     ` [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add Timur Tabi
  (?)
@ 2010-09-01 18:49       ` Liam Girdwood
  -1 siblings, 0 replies; 59+ messages in thread
From: Liam Girdwood @ 2010-09-01 18:49 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Julia Lawall, alsa-devel, Takashi Iwai, devicetree-discuss,
	Mark Brown, kernel-janitors, linux-kernel, Grant Likely

On Wed, 2010-09-01 at 13:39 -0500, Timur Tabi wrote:
> On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> > Add a call to of_node_put in the error handling code following a call to
> > of_parse_phandle.
> >
> > This patch also moves the existing call to of_node_put tothe end of the
> > error handling code, to make it possible to jump to of_node_put without
> > doing the other cleanup operations.  These appear to be disjoint
> > operations, so the ordering doesn't matter.
> 
> Acked-by: Timur Tabi <timur@freescale.com>

Acked-by: Liam Girdwood <lrg@slimlogic.uo.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk


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

* Re: [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
@ 2010-09-01 18:49       ` Liam Girdwood
  0 siblings, 0 replies; 59+ messages in thread
From: Liam Girdwood @ 2010-09-01 18:49 UTC (permalink / raw)
  To: Timur Tabi
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, Mark Brown,
	kernel-janitors, linux-kernel, Grant Likely, Julia Lawall

On Wed, 2010-09-01 at 13:39 -0500, Timur Tabi wrote:
> On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> > Add a call to of_node_put in the error handling code following a call to
> > of_parse_phandle.
> >
> > This patch also moves the existing call to of_node_put tothe end of the
> > error handling code, to make it possible to jump to of_node_put without
> > doing the other cleanup operations.  These appear to be disjoint
> > operations, so the ordering doesn't matter.
> 
> Acked-by: Timur Tabi <timur@freescale.com>

Acked-by: Liam Girdwood <lrg@slimlogic.uo.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

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

* Re: [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add
@ 2010-09-01 18:49       ` Liam Girdwood
  0 siblings, 0 replies; 59+ messages in thread
From: Liam Girdwood @ 2010-09-01 18:49 UTC (permalink / raw)
  To: Timur Tabi
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, Mark Brown,
	kernel-janitors, linux-kernel, Grant Likely, Julia Lawall

On Wed, 2010-09-01 at 13:39 -0500, Timur Tabi wrote:
> On Tue, Aug 31, 2010 at 10:48 AM, Julia Lawall <julia@diku.dk> wrote:
> > Add a call to of_node_put in the error handling code following a call to
> > of_parse_phandle.
> >
> > This patch also moves the existing call to of_node_put tothe end of the
> > error handling code, to make it possible to jump to of_node_put without
> > doing the other cleanup operations.  These appear to be disjoint
> > operations, so the ordering doesn't matter.
> 
> Acked-by: Timur Tabi <timur@freescale.com>

Acked-by: Liam Girdwood <lrg@slimlogic.uo.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk


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

* Re: [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
  2010-08-31 15:48   ` Julia Lawall
  (?)
@ 2010-09-02 15:30     ` Mark Brown
  -1 siblings, 0 replies; 59+ messages in thread
From: Mark Brown @ 2010-09-02 15:30 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Liam Girdwood, kernel-janitors, Jaroslav Kysela, Takashi Iwai,
	Grant Likely, alsa-devel, linux-kernel, devicetree-discuss

On Tue, Aug 31, 2010 at 05:48:56PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.

Applied, thanks.

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

* Re: [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak
@ 2010-09-02 15:30     ` Mark Brown
  0 siblings, 0 replies; 59+ messages in thread
From: Mark Brown @ 2010-09-02 15:30 UTC (permalink / raw)
  To: Julia Lawall
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, kernel-janitors,
	linux-kernel, Grant Likely, Liam Girdwood

On Tue, Aug 31, 2010 at 05:48:56PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.

Applied, thanks.

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

* Re: [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid
@ 2010-09-02 15:30     ` Mark Brown
  0 siblings, 0 replies; 59+ messages in thread
From: Mark Brown @ 2010-09-02 15:30 UTC (permalink / raw)
  To: Julia Lawall
  Cc: alsa-devel, Takashi Iwai, devicetree-discuss, kernel-janitors,
	linux-kernel, Grant Likely, Liam Girdwood

On Tue, Aug 31, 2010 at 05:48:56PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_parse_phandle.

Applied, thanks.

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
  2010-08-31 15:48   ` Julia Lawall
  (?)
@ 2010-09-08 19:59     ` Grant Likely
  -1 siblings, 0 replies; 59+ messages in thread
From: Grant Likely @ 2010-09-08 19:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Timur Tabi, kernel-janitors, linuxppc-dev, linux-kernel,
	devicetree-discuss

On Tue, Aug 31, 2010 at 05:48:55PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
> 
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Looks good to me.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> ---
>  drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
> index 3f4848e..38a5ef0 100644
> --- a/drivers/serial/ucc_uart.c
> +++ b/drivers/serial/ucc_uart.c
> @@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	ret = of_address_to_resource(np, 0, &res);
>  	if (ret) {
>  		dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
> -		kfree(qe_port);
> -		return ret;
> +		goto out_free;
>  	}
>  	if (!res.start) {
>  		dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  	qe_port->port.mapbase = res.start;
>  
> @@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (!iprop) {
>  		iprop = of_get_property(np, "device-id", NULL);
>  		if (!iprop) {
> -			kfree(qe_port);
>  			dev_err(&ofdev->dev, "UCC is unspecified in "
>  				"device tree\n");
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_free;
>  		}
>  	}
>  
>  	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
>  		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  	qe_port->ucc_num = *iprop - 1;
>  
> @@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	sprop = of_get_property(np, "rx-clock-name", NULL);
>  	if (!sprop) {
>  		dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  	qe_port->us_info.rx_clock = qe_clock_source(sprop);
>  	if ((qe_port->us_info.rx_clock < QE_BRG1) ||
>  	    (qe_port->us_info.rx_clock > QE_BRG16)) {
>  		dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  #ifdef LOOPBACK
> @@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	sprop = of_get_property(np, "tx-clock-name", NULL);
>  	if (!sprop) {
>  		dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  	qe_port->us_info.tx_clock = qe_clock_source(sprop);
>  #endif
>  	if ((qe_port->us_info.tx_clock < QE_BRG1) ||
>  	    (qe_port->us_info.tx_clock > QE_BRG16)) {
>  		dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  	/* Get the port number, numbered 0-3 */
>  	iprop = of_get_property(np, "port-number", NULL);
>  	if (!iprop) {
>  		dev_err(&ofdev->dev, "missing port-number in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  	qe_port->port.line = *iprop;
>  	if (qe_port->port.line >= UCC_MAX_UART) {
>  		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
>  			UCC_MAX_UART - 1);
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  
>  	qe_port->port.irq = irq_of_parse_and_map(np, 0);
>  	if (qe_port->port.irq == NO_IRQ) {
>  		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
>  		       qe_port->ucc_num + 1);
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  
>  	/*
> @@ -1373,8 +1372,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  		np = of_find_node_by_type(NULL, "qe");
>  		if (!np) {
>  			dev_err(&ofdev->dev, "could not find 'qe' node\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_free;
>  		}
>  	}
>  
> @@ -1382,8 +1381,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (!iprop) {
>  		dev_err(&ofdev->dev,
>  		       "missing brg-frequency in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_np;
>  	}
>  
>  	if (*iprop)
> @@ -1398,16 +1397,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  		if (!iprop) {
>  			dev_err(&ofdev->dev,
>  				"missing QE bus-frequency in device tree\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_np;
>  		}
>  		if (*iprop)
>  			qe_port->port.uartclk = *iprop / 2;
>  		else {
>  			dev_err(&ofdev->dev,
>  				"invalid QE bus-frequency in device tree\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_np;
>  		}
>  	}
>  
> @@ -1445,8 +1444,7 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (ret) {
>  		dev_err(&ofdev->dev, "could not add /dev/ttyQE%u\n",
>  		       qe_port->port.line);
> -		kfree(qe_port);
> -		return ret;
> +		goto out_np;
>  	}
>  
>  	dev_set_drvdata(&ofdev->dev, qe_port);
> @@ -1460,6 +1458,11 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	       SERIAL_QE_MINOR + qe_port->port.line);
>  
>  	return 0;
> +out_np:
> +	of_node_put(np);
> +out_free:
> +	kfree(qe_port);
> +	return ret;
>  }
>  
>  static int ucc_uart_remove(struct platform_device *ofdev)
> 

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to
@ 2010-09-08 19:59     ` Grant Likely
  0 siblings, 0 replies; 59+ messages in thread
From: Grant Likely @ 2010-09-08 19:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Timur Tabi, kernel-janitors, linuxppc-dev, linux-kernel,
	devicetree-discuss

On Tue, Aug 31, 2010 at 05:48:55PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
> 
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x = NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Looks good to me.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> ---
>  drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
> index 3f4848e..38a5ef0 100644
> --- a/drivers/serial/ucc_uart.c
> +++ b/drivers/serial/ucc_uart.c
> @@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	ret = of_address_to_resource(np, 0, &res);
>  	if (ret) {
>  		dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
> -		kfree(qe_port);
> -		return ret;
> +		goto out_free;
>  	}
>  	if (!res.start) {
>  		dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  	qe_port->port.mapbase = res.start;
>  
> @@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (!iprop) {
>  		iprop = of_get_property(np, "device-id", NULL);
>  		if (!iprop) {
> -			kfree(qe_port);
>  			dev_err(&ofdev->dev, "UCC is unspecified in "
>  				"device tree\n");
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_free;
>  		}
>  	}
>  
>  	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
>  		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  	qe_port->ucc_num = *iprop - 1;
>  
> @@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	sprop = of_get_property(np, "rx-clock-name", NULL);
>  	if (!sprop) {
>  		dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  	qe_port->us_info.rx_clock = qe_clock_source(sprop);
>  	if ((qe_port->us_info.rx_clock < QE_BRG1) ||
>  	    (qe_port->us_info.rx_clock > QE_BRG16)) {
>  		dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  #ifdef LOOPBACK
> @@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	sprop = of_get_property(np, "tx-clock-name", NULL);
>  	if (!sprop) {
>  		dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  	qe_port->us_info.tx_clock = qe_clock_source(sprop);
>  #endif
>  	if ((qe_port->us_info.tx_clock < QE_BRG1) ||
>  	    (qe_port->us_info.tx_clock > QE_BRG16)) {
>  		dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  	/* Get the port number, numbered 0-3 */
>  	iprop = of_get_property(np, "port-number", NULL);
>  	if (!iprop) {
>  		dev_err(&ofdev->dev, "missing port-number in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  	qe_port->port.line = *iprop;
>  	if (qe_port->port.line >= UCC_MAX_UART) {
>  		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
>  			UCC_MAX_UART - 1);
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  
>  	qe_port->port.irq = irq_of_parse_and_map(np, 0);
>  	if (qe_port->port.irq = NO_IRQ) {
>  		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
>  		       qe_port->ucc_num + 1);
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  
>  	/*
> @@ -1373,8 +1372,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  		np = of_find_node_by_type(NULL, "qe");
>  		if (!np) {
>  			dev_err(&ofdev->dev, "could not find 'qe' node\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_free;
>  		}
>  	}
>  
> @@ -1382,8 +1381,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (!iprop) {
>  		dev_err(&ofdev->dev,
>  		       "missing brg-frequency in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_np;
>  	}
>  
>  	if (*iprop)
> @@ -1398,16 +1397,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  		if (!iprop) {
>  			dev_err(&ofdev->dev,
>  				"missing QE bus-frequency in device tree\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_np;
>  		}
>  		if (*iprop)
>  			qe_port->port.uartclk = *iprop / 2;
>  		else {
>  			dev_err(&ofdev->dev,
>  				"invalid QE bus-frequency in device tree\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_np;
>  		}
>  	}
>  
> @@ -1445,8 +1444,7 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (ret) {
>  		dev_err(&ofdev->dev, "could not add /dev/ttyQE%u\n",
>  		       qe_port->port.line);
> -		kfree(qe_port);
> -		return ret;
> +		goto out_np;
>  	}
>  
>  	dev_set_drvdata(&ofdev->dev, qe_port);
> @@ -1460,6 +1458,11 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	       SERIAL_QE_MINOR + qe_port->port.line);
>  
>  	return 0;
> +out_np:
> +	of_node_put(np);
> +out_free:
> +	kfree(qe_port);
> +	return ret;
>  }
>  
>  static int ucc_uart_remove(struct platform_device *ofdev)
> 

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2010-09-08 19:59     ` Grant Likely
  0 siblings, 0 replies; 59+ messages in thread
From: Grant Likely @ 2010-09-08 19:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: linuxppc-dev, devicetree-discuss, kernel-janitors, Timur Tabi,
	linux-kernel

On Tue, Aug 31, 2010 at 05:48:55PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
> 
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Looks good to me.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> ---
>  drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
> index 3f4848e..38a5ef0 100644
> --- a/drivers/serial/ucc_uart.c
> +++ b/drivers/serial/ucc_uart.c
> @@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	ret = of_address_to_resource(np, 0, &res);
>  	if (ret) {
>  		dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
> -		kfree(qe_port);
> -		return ret;
> +		goto out_free;
>  	}
>  	if (!res.start) {
>  		dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  	qe_port->port.mapbase = res.start;
>  
> @@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (!iprop) {
>  		iprop = of_get_property(np, "device-id", NULL);
>  		if (!iprop) {
> -			kfree(qe_port);
>  			dev_err(&ofdev->dev, "UCC is unspecified in "
>  				"device tree\n");
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_free;
>  		}
>  	}
>  
>  	if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
>  		dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  	qe_port->ucc_num = *iprop - 1;
>  
> @@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	sprop = of_get_property(np, "rx-clock-name", NULL);
>  	if (!sprop) {
>  		dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  	qe_port->us_info.rx_clock = qe_clock_source(sprop);
>  	if ((qe_port->us_info.rx_clock < QE_BRG1) ||
>  	    (qe_port->us_info.rx_clock > QE_BRG16)) {
>  		dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  #ifdef LOOPBACK
> @@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	sprop = of_get_property(np, "tx-clock-name", NULL);
>  	if (!sprop) {
>  		dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  	qe_port->us_info.tx_clock = qe_clock_source(sprop);
>  #endif
>  	if ((qe_port->us_info.tx_clock < QE_BRG1) ||
>  	    (qe_port->us_info.tx_clock > QE_BRG16)) {
>  		dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
> -		kfree(qe_port);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_free;
>  	}
>  
>  	/* Get the port number, numbered 0-3 */
>  	iprop = of_get_property(np, "port-number", NULL);
>  	if (!iprop) {
>  		dev_err(&ofdev->dev, "missing port-number in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  	qe_port->port.line = *iprop;
>  	if (qe_port->port.line >= UCC_MAX_UART) {
>  		dev_err(&ofdev->dev, "port-number must be 0-%u\n",
>  			UCC_MAX_UART - 1);
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  
>  	qe_port->port.irq = irq_of_parse_and_map(np, 0);
>  	if (qe_port->port.irq == NO_IRQ) {
>  		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
>  		       qe_port->ucc_num + 1);
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_free;
>  	}
>  
>  	/*
> @@ -1373,8 +1372,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  		np = of_find_node_by_type(NULL, "qe");
>  		if (!np) {
>  			dev_err(&ofdev->dev, "could not find 'qe' node\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_free;
>  		}
>  	}
>  
> @@ -1382,8 +1381,8 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (!iprop) {
>  		dev_err(&ofdev->dev,
>  		       "missing brg-frequency in device tree\n");
> -		kfree(qe_port);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_np;
>  	}
>  
>  	if (*iprop)
> @@ -1398,16 +1397,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  		if (!iprop) {
>  			dev_err(&ofdev->dev,
>  				"missing QE bus-frequency in device tree\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_np;
>  		}
>  		if (*iprop)
>  			qe_port->port.uartclk = *iprop / 2;
>  		else {
>  			dev_err(&ofdev->dev,
>  				"invalid QE bus-frequency in device tree\n");
> -			kfree(qe_port);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_np;
>  		}
>  	}
>  
> @@ -1445,8 +1444,7 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	if (ret) {
>  		dev_err(&ofdev->dev, "could not add /dev/ttyQE%u\n",
>  		       qe_port->port.line);
> -		kfree(qe_port);
> -		return ret;
> +		goto out_np;
>  	}
>  
>  	dev_set_drvdata(&ofdev->dev, qe_port);
> @@ -1460,6 +1458,11 @@ static int ucc_uart_probe(struct platform_device *ofdev,
>  	       SERIAL_QE_MINOR + qe_port->port.line);
>  
>  	return 0;
> +out_np:
> +	of_node_put(np);
> +out_free:
> +	kfree(qe_port);
> +	return ret;
>  }
>  
>  static int ucc_uart_remove(struct platform_device *ofdev)
> 

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

* Re: [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
@ 2010-09-08 20:00     ` Grant Likely
  0 siblings, 0 replies; 59+ messages in thread
From: Grant Likely @ 2010-09-08 20:00 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Benjamin Herrenschmidt, kernel-janitors, Paul Mackerras,
	linuxppc-dev, linux-kernel, devicetree-discuss

On Tue, Aug 31, 2010 at 05:48:58PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_node_by_type.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> ---
>  arch/powerpc/platforms/chrp/nvram.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/chrp/nvram.c b/arch/powerpc/platforms/chrp/nvram.c
> index ba3588f..d3ceff0 100644
> --- a/arch/powerpc/platforms/chrp/nvram.c
> +++ b/arch/powerpc/platforms/chrp/nvram.c
> @@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
>  		return;
>  
>  	nbytes_p = of_get_property(nvram, "#bytes", &proplen);
> -	if (nbytes_p == NULL || proplen != sizeof(unsigned int))
> +	if (nbytes_p == NULL || proplen != sizeof(unsigned int)) {
> +		of_node_put(nvram);
>  		return;
> +	}
>  
>  	nvram_size = *nbytes_p;
>  
> 

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

* Re: [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
@ 2010-09-08 20:00     ` Grant Likely
  0 siblings, 0 replies; 59+ messages in thread
From: Grant Likely @ 2010-09-08 20:00 UTC (permalink / raw)
  To: Julia Lawall
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ

On Tue, Aug 31, 2010 at 05:48:58PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_node_by_type.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

> 
> ---
>  arch/powerpc/platforms/chrp/nvram.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/chrp/nvram.c b/arch/powerpc/platforms/chrp/nvram.c
> index ba3588f..d3ceff0 100644
> --- a/arch/powerpc/platforms/chrp/nvram.c
> +++ b/arch/powerpc/platforms/chrp/nvram.c
> @@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
>  		return;
>  
>  	nbytes_p = of_get_property(nvram, "#bytes", &proplen);
> -	if (nbytes_p == NULL || proplen != sizeof(unsigned int))
> +	if (nbytes_p == NULL || proplen != sizeof(unsigned int)) {
> +		of_node_put(nvram);
>  		return;
> +	}
>  
>  	nvram_size = *nbytes_p;
>  
> 

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

* Re: [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add
@ 2010-09-08 20:00     ` Grant Likely
  0 siblings, 0 replies; 59+ messages in thread
From: Grant Likely @ 2010-09-08 20:00 UTC (permalink / raw)
  To: Julia Lawall
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ

On Tue, Aug 31, 2010 at 05:48:58PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_node_by_type.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x = NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> ---
>  arch/powerpc/platforms/chrp/nvram.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/chrp/nvram.c b/arch/powerpc/platforms/chrp/nvram.c
> index ba3588f..d3ceff0 100644
> --- a/arch/powerpc/platforms/chrp/nvram.c
> +++ b/arch/powerpc/platforms/chrp/nvram.c
> @@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
>  		return;
>  
>  	nbytes_p = of_get_property(nvram, "#bytes", &proplen);
> -	if (nbytes_p = NULL || proplen != sizeof(unsigned int))
> +	if (nbytes_p = NULL || proplen != sizeof(unsigned int)) {
> +		of_node_put(nvram);
>  		return;
> +	}
>  
>  	nvram_size = *nbytes_p;
>  
> 

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

* Re: [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
@ 2010-09-08 20:00     ` Grant Likely
  0 siblings, 0 replies; 59+ messages in thread
From: Grant Likely @ 2010-09-08 20:00 UTC (permalink / raw)
  To: Julia Lawall
  Cc: devicetree-discuss, kernel-janitors, linux-kernel,
	Paul Mackerras, linuxppc-dev

On Tue, Aug 31, 2010 at 05:48:58PM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_node_by_type.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>   ... when != of_node_put(x)
>       when != if (...) { ... of_node_put(x); ... }
> (
>   return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> ---
>  arch/powerpc/platforms/chrp/nvram.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/chrp/nvram.c b/arch/powerpc/platforms/chrp/nvram.c
> index ba3588f..d3ceff0 100644
> --- a/arch/powerpc/platforms/chrp/nvram.c
> +++ b/arch/powerpc/platforms/chrp/nvram.c
> @@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
>  		return;
>  
>  	nbytes_p = of_get_property(nvram, "#bytes", &proplen);
> -	if (nbytes_p == NULL || proplen != sizeof(unsigned int))
> +	if (nbytes_p == NULL || proplen != sizeof(unsigned int)) {
> +		of_node_put(nvram);
>  		return;
> +	}
>  
>  	nvram_size = *nbytes_p;
>  
> 

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2011-03-15 15:51     ` Kumar Gala
  0 siblings, 0 replies; 59+ messages in thread
From: Kumar Gala @ 2011-03-15 15:51 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Timur Tabi, linuxppc-dev, devicetree-discuss, kernel-janitors,
	linux-kernel


On Aug 31, 2010, at 10:48 AM, Julia Lawall wrote:

> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
> 
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>  ... when != of_node_put(x)
>      when != if (...) { ... of_node_put(x); ... }
> (
>  return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 32 deletions(-)

applied

- k

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2011-03-15 15:51     ` Kumar Gala
  0 siblings, 0 replies; 59+ messages in thread
From: Kumar Gala @ 2011-03-15 15:51 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Timur Tabi,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA


On Aug 31, 2010, at 10:48 AM, Julia Lawall wrote:

> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
> 
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
>  ... when != of_node_put(x)
>      when != if (...) { ... of_node_put(x); ... }
> (
>  return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> 
> ---
> drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 32 deletions(-)

applied

- k

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2011-03-15 15:51     ` Kumar Gala
  0 siblings, 0 replies; 59+ messages in thread
From: Kumar Gala @ 2011-03-15 15:51 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Timur Tabi,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA


On Aug 31, 2010, at 10:48 AM, Julia Lawall wrote:

> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node or of_find_node_by_type.
> 
> This patch also substantially reorganizes the error handling code in the
> function, to that it is possible first to jump to code that frees qe_port
> and then to jump to code that also puts np.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
> 
> *x = 
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x = NULL) S
> <... when != x = E
> *if (...) {
>  ... when != of_node_put(x)
>      when != if (...) { ... of_node_put(x); ... }
> (
>  return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 = x;
> |
> of_node_put(x);
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> drivers/serial/ucc_uart.c |   67 ++++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 32 deletions(-)

applied

- k

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

* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
@ 2011-03-15 15:51     ` Kumar Gala
  0 siblings, 0 replies; 59+ messages in thread
From: Kumar Gala @ 2011-03-15 15:51 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, devicetree-discuss, linuxppc-dev, Timur Tabi,
	linux-kernel


On Aug 31, 2010, at 10:48 AM, Julia Lawall wrote:

> Add a call to of_node_put in the error handling code following a call =
to
> of_find_compatible_node or of_find_node_by_type.
>=20
> This patch also substantially reorganizes the error handling code in =
the
> function, to that it is possible first to jump to code that frees =
qe_port
> and then to jump to code that also puts np.
>=20
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>=20
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1,E2;
> statement S;
> @@
>=20
> *x =3D=20
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> |of_find_node_by_type
> |of_find_node_with_property
> |of_find_matching_node
> |of_parse_phandle
> )(...);
> ...
> if (x =3D=3D NULL) S
> <... when !=3D x =3D E
> *if (...) {
>  ... when !=3D of_node_put(x)
>      when !=3D if (...) { ... of_node_put(x); ... }
> (
>  return <+...x...+>;
> |
> *  return ...;
> )
> }
> ...>
> (
> E2 =3D x;
> |
> of_node_put(x);
> )
> // </smpl>
>=20
> Signed-off-by: Julia Lawall <julia@diku.dk>
>=20
> ---
> drivers/serial/ucc_uart.c |   67 =
++++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 32 deletions(-)

applied

- k=

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

end of thread, other threads:[~2011-03-15 15:53 UTC | newest]

Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-31 15:48 [PATCH 0/4] Add of_node_put to avoid memory leak Julia Lawall
2010-08-31 15:48 ` Julia Lawall
2010-08-31 15:48 ` Julia Lawall
2010-08-31 15:48 ` [PATCH 1/4] drivers/serial/ucc_uart.c: " Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-09-01 18:40   ` Timur Tabi
2010-09-01 18:40     ` Timur Tabi
2010-09-01 18:40     ` [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid Timur Tabi
2010-09-01 18:40     ` [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak Timur Tabi
2010-09-08 19:59   ` Grant Likely
2010-09-08 19:59     ` Grant Likely
2010-09-08 19:59     ` [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to Grant Likely
2011-03-15 15:51   ` [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak Kumar Gala
2011-03-15 15:51     ` Kumar Gala
2011-03-15 15:51     ` Kumar Gala
2011-03-15 15:51     ` Kumar Gala
2010-08-31 15:48 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-08-31 15:59   ` Mark Brown
2010-08-31 15:59     ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid Mark Brown
2010-08-31 15:59     ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak Mark Brown
2010-09-01 18:39   ` [alsa-devel] " Timur Tabi
2010-09-01 18:39     ` [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add Timur Tabi
2010-09-01 18:49     ` [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak Liam Girdwood
2010-09-01 18:49       ` [alsa-devel] [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add Liam Girdwood
2010-09-01 18:49       ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak Liam Girdwood
2010-09-02 15:30   ` Mark Brown
2010-09-02 15:30     ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid Mark Brown
2010-09-02 15:30     ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: Add of_node_put to avoid memory leak Mark Brown
2010-08-31 15:48 ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: " Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-08-31 16:51   ` Wolfram Sang
2010-08-31 16:51     ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add Wolfram Sang
2010-08-31 16:51     ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak Wolfram Sang
2010-08-31 16:55     ` Julia Lawall
2010-08-31 16:55       ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put Julia Lawall
2010-08-31 17:44     ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak Julia Lawall
2010-08-31 17:44       ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put Julia Lawall
2010-08-31 17:44       ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak Julia Lawall
2010-08-31 19:10       ` Wolfram Sang
2010-08-31 19:10         ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add Wolfram Sang
2010-08-31 19:10         ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak Wolfram Sang
2010-08-31 19:22         ` Wolfgang Grandegger
2010-08-31 19:22           ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put Wolfgang Grandegger
2010-08-31 19:22           ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak Wolfgang Grandegger
2010-09-01 17:59           ` David Miller
2010-09-01 17:59             ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add David Miller
2010-09-01 17:59             ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak David Miller
2010-08-31 15:48 ` [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: " Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-08-31 15:48   ` Julia Lawall
2010-09-08 20:00   ` Grant Likely
2010-09-08 20:00     ` Grant Likely
2010-09-08 20:00     ` [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add Grant Likely
2010-09-08 20:00     ` [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak Grant Likely

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.