Message ID | Pine.LNX.4.64.1008311943280.2668@ask.diku.dk |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote: > Add a call to of_node_put in the error handling code following a call to > of_find_matching_node. > > This patch also moves the existing call to of_node_put after the call to > iounmap in the error handling code, to make it possible to jump to > of_node_put without doing iounmap. These appear to be disjoint operations, > so the ordering doesn't matter. > > This patch furthermore changes the -ENODEV result in the error handling > code for of_find_matching_node to a return of 0, as found in the error > handling code for of_iomap, because the return type of the function is > unsigned. > > The semantic match that finds this problem is as follows: > (http://coccinelle.lip6.fr/) > > // <smpl> > @r exists@ > local idexpression x; > expression E,E1,E2; > statement S; > @@ > > *x = > (of_find_node_by_path > |of_find_node_by_name > |of_find_node_by_phandle > |of_get_parent > |of_get_next_parent > |of_get_next_child > |of_find_compatible_node > |of_match_node > |of_find_node_by_type > |of_find_node_with_property > |of_find_matching_node > |of_parse_phandle > )(...); > ... > if (x == NULL) S > <... when != x = E > *if (...) { > ... when != of_node_put(x) > when != if (...) { ... of_node_put(x); ... } > ( > return <+...x...+>; > | > * return ...; > ) > } > ...> > ( > E2 = x; > | > of_node_put(x); > ) > // </smpl> > > Signed-off-by: Julia Lawall <julia@diku.dk> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de> > > --- > drivers/net/can/mscan/mpc5xxx_can.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c > --- a/drivers/net/can/mscan/mpc5xxx_can.c > +++ b/drivers/net/can/mscan/mpc5xxx_can.c > @@ -143,12 +143,12 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev, > np_clock = of_find_matching_node(NULL, mpc512x_clock_ids); > if (!np_clock) { > dev_err(&ofdev->dev, "couldn't find clock node\n"); > - return -ENODEV; > + return 0; > } > clockctl = of_iomap(np_clock, 0); > if (!clockctl) { > dev_err(&ofdev->dev, "couldn't map clock registers\n"); > - return 0; > + goto exit_put; > } > > /* Determine the MSCAN device index from the physical address */ > @@ -233,9 +233,9 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev, > clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv); > > exit_unmap: > - of_node_put(np_clock); > iounmap(clockctl); > - > +exit_put: > + of_node_put(np_clock); > return freq; > } > #else /* !CONFIG_PPC_MPC512x */
On 08/31/2010 09:10 PM, Wolfram Sang wrote: > On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote: >> Add a call to of_node_put in the error handling code following a call to >> of_find_matching_node. >> >> This patch also moves the existing call to of_node_put after the call to >> iounmap in the error handling code, to make it possible to jump to >> of_node_put without doing iounmap. These appear to be disjoint operations, >> so the ordering doesn't matter. >> >> This patch furthermore changes the -ENODEV result in the error handling >> code for of_find_matching_node to a return of 0, as found in the error >> handling code for of_iomap, because the return type of the function is >> unsigned. >> >> The semantic match that finds this problem is as follows: >> (http://coccinelle.lip6.fr/) >> >> // <smpl> >> @r exists@ >> local idexpression x; >> expression E,E1,E2; >> statement S; >> @@ >> >> *x = >> (of_find_node_by_path >> |of_find_node_by_name >> |of_find_node_by_phandle >> |of_get_parent >> |of_get_next_parent >> |of_get_next_child >> |of_find_compatible_node >> |of_match_node >> |of_find_node_by_type >> |of_find_node_with_property >> |of_find_matching_node >> |of_parse_phandle >> )(...); >> ... >> if (x == NULL) S >> <... when != x = E >> *if (...) { >> ... when != of_node_put(x) >> when != if (...) { ... of_node_put(x); ... } >> ( >> return <+...x...+>; >> | >> * return ...; >> ) >> } >> ...> >> ( >> E2 = x; >> | >> of_node_put(x); >> ) >> // </smpl> >> >> Signed-off-by: Julia Lawall <julia@diku.dk> > > Reviewed-by: Wolfram Sang <w.sang@pengutronix.de> Acked-by: Wolfgang Grandegger <wg@grandegger.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Wolfgang Grandegger <wg@grandegger.com> Date: Tue, 31 Aug 2010 21:22:07 +0200 > On 08/31/2010 09:10 PM, Wolfram Sang wrote: >> On Tue, Aug 31, 2010 at 07:44:00PM +0200, Julia Lawall wrote: >>> Add a call to of_node_put in the error handling code following a call to >>> of_find_matching_node. >>> >>> This patch also moves the existing call to of_node_put after the call to >>> iounmap in the error handling code, to make it possible to jump to >>> of_node_put without doing iounmap. These appear to be disjoint operations, >>> so the ordering doesn't matter. >>> >>> This patch furthermore changes the -ENODEV result in the error handling >>> code for of_find_matching_node to a return of 0, as found in the error >>> handling code for of_iomap, because the return type of the function is >>> unsigned. >>> >>> The semantic match that finds this problem is as follows: >>> (http://coccinelle.lip6.fr/) ... >>> Signed-off-by: Julia Lawall <julia@diku.dk> >> >> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de> > > Acked-by: Wolfgang Grandegger <wg@grandegger.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */
Add a call to of_node_put in the error handling code following a call to of_find_matching_node. This patch also moves the existing call to of_node_put after the call to iounmap in the error handling code, to make it possible to jump to of_node_put without doing iounmap. These appear to be disjoint operations, so the ordering doesn't matter. This patch furthermore changes the -ENODEV result in the error handling code for of_find_matching_node to a return of 0, as found in the error handling code for of_iomap, because the return type of the function is unsigned. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // <smpl> @r exists@ local idexpression x; expression E,E1,E2; statement S; @@ *x = (of_find_node_by_path |of_find_node_by_name |of_find_node_by_phandle |of_get_parent |of_get_next_parent |of_get_next_child |of_find_compatible_node |of_match_node |of_find_node_by_type |of_find_node_with_property |of_find_matching_node |of_parse_phandle )(...); ... if (x == NULL) S <... when != x = E *if (...) { ... when != of_node_put(x) when != if (...) { ... of_node_put(x); ... } ( return <+...x...+>; | * return ...; ) } ...> ( E2 = x; | of_node_put(x); ) // </smpl> Signed-off-by: Julia Lawall <julia@diku.dk> --- drivers/net/can/mscan/mpc5xxx_can.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html