* [PATCH 0/4] Add of_node_put to avoid memory leak
@ 2010-08-31 15:48 Julia Lawall
[not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>
` (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[parent not found: <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>]
* [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 2010-09-08 19:59 ` Grant Likely [not found] ` <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org> 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
* 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 @ 2010-09-08 19:59 ` Grant Likely [not found] ` <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org> 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
[parent not found: <1283269738-14612-2-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>]
* 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 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
* [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
[parent not found: <1283269738-14612-4-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>]
* 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
[parent not found: <20100831165153.GB28015-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* 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
[parent not found: <Pine.LNX.4.64.1008311943280.2668-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>]
* 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
[parent not found: <20100831191014.GA9603-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* 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
[parent not found: <4C7D565F.307-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>]
* 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
* [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 [not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org> @ 2010-08-31 15:48 ` Julia Lawall 2010-08-31 15:59 ` Mark Brown ` (2 more replies) 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
* 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: [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 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
* [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 [not found] ` <1283269738-14612-1-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org> 2010-08-31 15:48 ` [PATCH 2/4] sound/soc/fsl/p1022_ds.c: " Julia Lawall @ 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
[parent not found: <1283269738-14612-5-git-send-email-julia-dAYI7NvHqcQ@public.gmane.org>]
* 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
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
[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-09-08 19:59 ` Grant Likely
[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-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 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
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).