These patches introduce calls to of_node_put after various functions that
call of_node_get.
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 <[email protected]>
---
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;
}
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 <[email protected]>
---
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;
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 <[email protected]>
---
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 */
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 <[email protected]>
---
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)
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 <[email protected]>
>
> ---
> 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;
> }
>
>
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 <[email protected]>
>
> 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 <[email protected]>
>
> ---
> 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 [email protected]
> 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/ |
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
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 <[email protected]>
---
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 */
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 <[email protected]>
Reviewed-by: Wolfram Sang <[email protected]>
>
> ---
> 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/ |
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 <[email protected]>
>
> Reviewed-by: Wolfram Sang <[email protected]>
Acked-by: Wolfgang Grandegger <[email protected]>
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 <[email protected]>
>
> ---
> drivers/serial/ucc_uart.c | 67 ++++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 32 deletions(-)
applied
- k-