* [PATCH 0/4] Add of_node_put to avoid memory leak
@ 2010-08-31 15:48 Julia Lawall
2010-08-31 15:48 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall
` (2 more replies)
0 siblings, 3 replies; 19+ 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] 19+ messages in thread
* [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
[not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
@ 2010-08-31 15:48 ` Julia Lawall
[not found] ` <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-09-08 19:59 ` Grant Likely
2010-08-31 15:48 ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: " Julia Lawall
1 sibling, 2 replies; 19+ 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 related [flat|nested] 19+ 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 [PATCH 0/4] Add of_node_put to avoid memory leak Julia Lawall
@ 2010-08-31 15:48 ` Julia Lawall
2010-08-31 15:59 ` Mark Brown
` (2 more replies)
[not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-08-31 15:48 ` [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: " Julia Lawall
2 siblings, 3 replies; 19+ 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 related [flat|nested] 19+ messages in thread
* [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
[not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-08-31 15:48 ` [PATCH 1/4] drivers/serial/ucc_uart.c: " Julia Lawall
@ 2010-08-31 15:48 ` Julia Lawall
[not found] ` <1283269738-14612-4-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
1 sibling, 1 reply; 19+ 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 related [flat|nested] 19+ 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 [PATCH 0/4] Add of_node_put to avoid memory leak Julia Lawall
2010-08-31 15:48 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall
[not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
@ 2010-08-31 15:48 ` Julia Lawall
[not found] ` <1283269738-14612-5-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2 siblings, 1 reply; 19+ 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 related [flat|nested] 19+ 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 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall
@ 2010-08-31 15:59 ` Mark Brown
2010-09-01 18:39 ` [alsa-devel] " Timur Tabi
2010-09-02 15:30 ` Mark Brown
2 siblings, 0 replies; 19+ 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] 19+ messages in thread
* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
[not found] ` <1283269738-14612-4-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
@ 2010-08-31 16:51 ` Wolfram Sang
2010-08-31 16:55 ` Julia Lawall
[not found] ` <20100831165153.GB28015-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
0 siblings, 2 replies; 19+ 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] 19+ 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
@ 2010-08-31 16:55 ` Julia Lawall
[not found] ` <20100831165153.GB28015-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
1 sibling, 0 replies; 19+ 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] 19+ messages in thread
* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
[not found] ` <20100831165153.GB28015-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
@ 2010-08-31 17:44 ` Julia Lawall
[not found] ` <Pine.LNX.4.64.1008311943280.2668-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
[not found] ` <Pine.LNX.4.64.1008311943280.2668-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
@ 2010-08-31 19:10 ` Wolfram Sang
[not found] ` <20100831191014.GA9603-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
[not found] ` <20100831191014.GA9603-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
@ 2010-08-31 19:22 ` Wolfgang Grandegger
[not found] ` <4C7D565F.307-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: Add of_node_put to avoid memory leak
[not found] ` <4C7D565F.307-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
@ 2010-09-01 17:59 ` David Miller
0 siblings, 0 replies; 19+ 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] 19+ 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 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall
2010-08-31 15:59 ` Mark Brown
@ 2010-09-01 18:39 ` Timur Tabi
2010-09-01 18:49 ` Liam Girdwood
2010-09-02 15:30 ` Mark Brown
2 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
[not found] ` <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
@ 2010-09-01 18:40 ` Timur Tabi
2011-03-15 15:51 ` Kumar Gala
1 sibling, 0 replies; 19+ 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] 19+ 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:39 ` [alsa-devel] " Timur Tabi
@ 2010-09-01 18:49 ` Liam Girdwood
0 siblings, 0 replies; 19+ 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] 19+ 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 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall
2010-08-31 15:59 ` Mark Brown
2010-09-01 18:39 ` [alsa-devel] " Timur Tabi
@ 2010-09-02 15:30 ` Mark Brown
2 siblings, 0 replies; 19+ 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] 19+ 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 ` [PATCH 1/4] drivers/serial/ucc_uart.c: " Julia Lawall
[not found] ` <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
@ 2010-09-08 19:59 ` Grant Likely
1 sibling, 0 replies; 19+ 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] 19+ messages in thread
* Re: [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
[not found] ` <1283269738-14612-5-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
@ 2010-09-08 20:00 ` Grant Likely
0 siblings, 0 replies; 19+ 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] 19+ messages in thread
* Re: [PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
[not found] ` <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-09-01 18:40 ` Timur Tabi
@ 2011-03-15 15:51 ` Kumar Gala
1 sibling, 0 replies; 19+ 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] 19+ messages in thread
end of thread, other threads:[~2011-03-15 15:51 UTC | newest]
Thread overview: 19+ 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 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall
2010-08-31 15:59 ` Mark Brown
2010-09-01 18:39 ` [alsa-devel] " Timur Tabi
2010-09-01 18:49 ` Liam Girdwood
2010-09-02 15:30 ` Mark Brown
[not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-08-31 15:48 ` [PATCH 1/4] drivers/serial/ucc_uart.c: " Julia Lawall
[not found] ` <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-09-01 18:40 ` Timur Tabi
2011-03-15 15:51 ` Kumar Gala
2010-09-08 19:59 ` Grant Likely
2010-08-31 15:48 ` [PATCH 3/4] drivers/net/can/mscan/mpc5xxx_can.c: " Julia Lawall
[not found] ` <1283269738-14612-4-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-08-31 16:51 ` Wolfram Sang
2010-08-31 16:55 ` Julia Lawall
[not found] ` <20100831165153.GB28015-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-08-31 17:44 ` Julia Lawall
[not found] ` <Pine.LNX.4.64.1008311943280.2668-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
2010-08-31 19:10 ` Wolfram Sang
[not found] ` <20100831191014.GA9603-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-08-31 19:22 ` Wolfgang Grandegger
[not found] ` <4C7D565F.307-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2010-09-01 17:59 ` David Miller
2010-08-31 15:48 ` [PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: " Julia Lawall
[not found] ` <1283269738-14612-5-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
2010-09-08 20:00 ` Grant Likely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).