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_find_node_by_path.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
local idexpression x;
expression E,E1;
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
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
arch/powerpc/platforms/maple/setup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c
index 3fff8d9..fe34c3d 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -358,6 +358,7 @@ static int __init maple_cpc925_edac_setup(void)
model = (const unsigned char *)of_get_property(np, "model", NULL);
if (!model) {
printk(KERN_ERR "%s: Unabel to get model info\n", __func__);
+ of_node_put(np);
return -ENODEV;
}
Add calls to of_node_put in the error handling code following calls to
of_find_node_by_path and of_find_node_by_phandle.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
local idexpression x;
expression E,E1;
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
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
arch/powerpc/platforms/cell/ras.c | 4 +++-
arch/powerpc/platforms/cell/spider-pic.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/cell/ras.c b/arch/powerpc/platforms/cell/ras.c
index 1d3c4ef..5ec1e47 100644
--- a/arch/powerpc/platforms/cell/ras.c
+++ b/arch/powerpc/platforms/cell/ras.c
@@ -173,8 +173,10 @@ static int __init cbe_ptcal_enable(void)
return -ENODEV;
size = of_get_property(np, "ibm,cbe-ptcal-size", NULL);
- if (!size)
+ if (!size) {
+ of_node_put(np);
return -ENODEV;
+ }
pr_debug("%s: enabling PTCAL, size = 0x%x\n", __func__, *size);
order = get_order(*size);
diff --git a/arch/powerpc/platforms/cell/spider-pic.c b/arch/powerpc/platforms/cell/spider-pic.c
index 5876e88..3f2e557 100644
--- a/arch/powerpc/platforms/cell/spider-pic.c
+++ b/arch/powerpc/platforms/cell/spider-pic.c
@@ -258,8 +258,10 @@ static unsigned int __init spider_find_cascade_and_node(struct spider_pic *pic)
return NO_IRQ;
imap += intsize + 1;
tmp = of_get_property(iic, "#interrupt-cells", NULL);
- if (tmp == NULL)
+ if (tmp == NULL) {
+ of_node_put(iic);
return NO_IRQ;
+ }
intsize = *tmp;
/* Assume unit is last entry of interrupt specifier */
unit = imap[intsize - 1];
Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
local idexpression x;
expression E,E1;
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
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
arch/powerpc/sysdev/qe_lib/qe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index 3da8014..90020de 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -640,6 +640,7 @@ unsigned int qe_get_num_of_snums(void)
if ((num_of_snums < 28) || (num_of_snums > QE_NUM_OF_SNUM)) {
/* No QE ever has fewer than 28 SNUMs */
pr_err("QE: number of snum is invalid\n");
+ of_node_put(qe);
return -EINVAL;
}
}
Add a call to of_node_put in the error handling code following a call to
of_find_node_by_phandle.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
local idexpression x;
expression E,E1;
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
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
arch/powerpc/platforms/powermac/pfunc_core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powermac/pfunc_core.c b/arch/powerpc/platforms/powermac/pfunc_core.c
index cec6359..b0c3777 100644
--- a/arch/powerpc/platforms/powermac/pfunc_core.c
+++ b/arch/powerpc/platforms/powermac/pfunc_core.c
@@ -837,8 +837,10 @@ struct pmf_function *__pmf_find_function(struct device_node *target,
return NULL;
find_it:
dev = pmf_find_device(actor);
- if (dev == NULL)
- return NULL;
+ if (dev == NULL) {
+ result = NULL;
+ goto out;
+ }
list_for_each_entry(func, &dev->functions, link) {
if (name && strcmp(name, func->name))
@@ -850,8 +852,9 @@ struct pmf_function *__pmf_find_function(struct device_node *target,
result = func;
break;
}
- of_node_put(actor);
pmf_put_device(dev);
+out:
+ of_node_put(actor);
return result;
}
Add a call to of_node_put in the error handling code following a call to
of_find_node_by_path.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
local idexpression x;
expression E,E1;
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
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/macintosh/via-pmu-led.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/via-pmu-led.c b/drivers/macintosh/via-pmu-led.c
index d242976..19c3718 100644
--- a/drivers/macintosh/via-pmu-led.c
+++ b/drivers/macintosh/via-pmu-led.c
@@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
if (dt == NULL)
return -ENODEV;
model = of_get_property(dt, "model", NULL);
- if (model == NULL)
+ if (model == NULL) {
+ of_node_put(dt);
return -ENODEV;
+ }
if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
strncmp(model, "iBook", strlen("iBook")) != 0 &&
strcmp(model, "PowerMac7,2") != 0 &&
Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
local idexpression x;
expression E,E1;
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
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/mtd/nand/mpc5121_nfc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
index df0c1da..f4610bc 100644
--- a/drivers/mtd/nand/mpc5121_nfc.c
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -568,6 +568,7 @@ static int mpc5121_nfc_read_hw_config(struct mtd_info *mtd)
uint rcw_width;
uint rcwh;
uint romloc, ps;
+ int ret = 0;
rmnode = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-reset");
if (!rmnode) {
@@ -579,7 +580,8 @@ static int mpc5121_nfc_read_hw_config(struct mtd_info *mtd)
rm = of_iomap(rmnode, 0);
if (!rm) {
dev_err(prv->dev, "Error mapping reset module node!\n");
- return -EBUSY;
+ ret = -EBUSY;
+ goto out;
}
rcwh = in_be32(&rm->rcwhr);
@@ -628,8 +630,9 @@ static int mpc5121_nfc_read_hw_config(struct mtd_info *mtd)
rcw_width * 8, rcw_pagesize,
rcw_sparesize);
iounmap(rm);
+out:
of_node_put(rmnode);
- return 0;
+ return ret;
}
/* Free driver resources */
Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
local idexpression x;
expression E,E1;
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
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/serial/mpc52xx_uart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
index 8dedb26..c4399e2 100644
--- a/drivers/serial/mpc52xx_uart.c
+++ b/drivers/serial/mpc52xx_uart.c
@@ -500,6 +500,7 @@ static int __init mpc512x_psc_fifoc_init(void)
psc_fifoc = of_iomap(np, 0);
if (!psc_fifoc) {
pr_err("%s: Can't map FIFOC\n", __func__);
+ of_node_put(np);
return -ENODEV;
}
On Sun, Aug 29, 2010 at 11:52:41AM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1;
> 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
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
> ... when != of_node_put(x)
> when != if (...) { ... of_node_put(x); ... }
> (
> return <+...x...+>;
> |
> * return ...;
> )
> }
> ...>
> of_node_put(x);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
Acked-by: Wolfram Sang <[email protected]>
adding ppc-list to CC.
>
> ---
> drivers/serial/mpc52xx_uart.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
> index 8dedb26..c4399e2 100644
> --- a/drivers/serial/mpc52xx_uart.c
> +++ b/drivers/serial/mpc52xx_uart.c
> @@ -500,6 +500,7 @@ static int __init mpc512x_psc_fifoc_init(void)
> psc_fifoc = of_iomap(np, 0);
> if (!psc_fifoc) {
> pr_err("%s: Can't map FIFOC\n", __func__);
> + of_node_put(np);
> return -ENODEV;
> }
>
>
> --
> 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 Sun, Aug 29, 2010 at 11:52:42AM +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1;
> 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
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
> ... when != of_node_put(x)
> when != if (...) { ... of_node_put(x); ... }
> (
> return <+...x...+>;
> |
> * return ...;
> )
> }
> ...>
> of_node_put(x);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
Acked-by: Wolfram Sang <[email protected]>
adding ppc-list to cc.
>
> ---
> drivers/mtd/nand/mpc5121_nfc.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
> index df0c1da..f4610bc 100644
> --- a/drivers/mtd/nand/mpc5121_nfc.c
> +++ b/drivers/mtd/nand/mpc5121_nfc.c
> @@ -568,6 +568,7 @@ static int mpc5121_nfc_read_hw_config(struct mtd_info *mtd)
> uint rcw_width;
> uint rcwh;
> uint romloc, ps;
> + int ret = 0;
>
> rmnode = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-reset");
> if (!rmnode) {
> @@ -579,7 +580,8 @@ static int mpc5121_nfc_read_hw_config(struct mtd_info *mtd)
> rm = of_iomap(rmnode, 0);
> if (!rm) {
> dev_err(prv->dev, "Error mapping reset module node!\n");
> - return -EBUSY;
> + ret = -EBUSY;
> + goto out;
> }
>
> rcwh = in_be32(&rm->rcwhr);
> @@ -628,8 +630,9 @@ static int mpc5121_nfc_read_hw_config(struct mtd_info *mtd)
> rcw_width * 8, rcw_pagesize,
> rcw_sparesize);
> iounmap(rm);
> +out:
> of_node_put(rmnode);
> - return 0;
> + return ret;
> }
>
> /* Free driver resources */
>
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Sun, 2010-08-29 at 11:52 +0200, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
Pushed to l2-mtd-2.6.git / master, thanks.
--
Best Regards,
Artem Bityutskiy (Битюцкий Артём)
Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node.
Acked-by: Timur Tabi <[email protected]>
Thanks, Julia. Your work in finding these kinds of bugs is very much
appreciated.
Julia Lawall schrieb:
> Add a call to of_node_put in the error handling code following a call to
> of_find_node_by_path.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1;
> 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
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
> ... when != of_node_put(x)
> when != if (...) { ... of_node_put(x); ... }
> (
> return <+...x...+>;
> |
> * return ...;
> )
> }
> ...>
> of_node_put(x);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> drivers/macintosh/via-pmu-led.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/macintosh/via-pmu-led.c b/drivers/macintosh/via-pmu-led.c
> index d242976..19c3718 100644
> --- a/drivers/macintosh/via-pmu-led.c
> +++ b/drivers/macintosh/via-pmu-led.c
> @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> if (dt == NULL)
> return -ENODEV;
> model = of_get_property(dt, "model", NULL);
> - if (model == NULL)
> + if (model == NULL) {
> + of_node_put(dt);
> return -ENODEV;
> + }
> if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> strncmp(model, "iBook", strlen("iBook")) != 0 &&
> strcmp(model, "PowerMac7,2") != 0 &&
>
is there any rule that says when to use strncmp ? it seems perfecly valid to use strcpy here
(what is done in the last cmp).
re,
wh
On Tue, 31 Aug 2010, walter harms wrote:
>
>
> Julia Lawall schrieb:
> > Add a call to of_node_put in the error handling code following a call to
> > of_find_node_by_path.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @r exists@
> > local idexpression x;
> > expression E,E1;
> > 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
> > )(...);
> > ...
> > if (x == NULL) S
> > <... when != x = E
> > *if (...) {
> > ... when != of_node_put(x)
> > when != if (...) { ... of_node_put(x); ... }
> > (
> > return <+...x...+>;
> > |
> > * return ...;
> > )
> > }
> > ...>
> > of_node_put(x);
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <[email protected]>
> >
> > ---
> > drivers/macintosh/via-pmu-led.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/macintosh/via-pmu-led.c b/drivers/macintosh/via-pmu-led.c
> > index d242976..19c3718 100644
> > --- a/drivers/macintosh/via-pmu-led.c
> > +++ b/drivers/macintosh/via-pmu-led.c
> > @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> > if (dt == NULL)
> > return -ENODEV;
> > model = of_get_property(dt, "model", NULL);
> > - if (model == NULL)
> > + if (model == NULL) {
> > + of_node_put(dt);
> > return -ENODEV;
> > + }
> > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> > strncmp(model, "iBook", strlen("iBook")) != 0 &&
> > strcmp(model, "PowerMac7,2") != 0 &&
> >
>
> is there any rule that says when to use strncmp ? it seems perfecly valid to use strcpy here
> (what is done in the last cmp).
Perhaps there are some characters after eg PowerBook that one doesn't want
to compare with?
julia
On Tue, Aug 31, 2010 at 06:08:19PM +0200, Julia Lawall wrote:
> On Tue, 31 Aug 2010, walter harms wrote:
> > > @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> > > if (dt == NULL)
> > > return -ENODEV;
> > > model = of_get_property(dt, "model", NULL);
> > > - if (model == NULL)
> > > + if (model == NULL) {
> > > + of_node_put(dt);
> > > return -ENODEV;
> > > + }
> > > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> > > strncmp(model, "iBook", strlen("iBook")) != 0 &&
> > > strcmp(model, "PowerMac7,2") != 0 &&
> > >
> >
> > is there any rule that says when to use strncmp ? it seems perfecly valid to use strcpy here
> > (what is done in the last cmp).
>
> Perhaps there are some characters after eg PowerBook that one doesn't want
> to compare with?
Yes. The model property on powermacs always has the version number. strncmp makes sure that *all* PowerBooks and iBooks are matched.
g.
On Tue, Aug 31, 2010 at 18:08 +0200, Julia Lawall wrote:
> On Tue, 31 Aug 2010, walter harms wrote:
>
> >
> >
> > Julia Lawall schrieb:
> > > Add a call to of_node_put in the error handling code following a call to
> > > of_find_node_by_path.
[...]
> > > --- a/drivers/macintosh/via-pmu-led.c
> > > +++ b/drivers/macintosh/via-pmu-led.c
> > > @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> > > if (dt == NULL)
> > > return -ENODEV;
> > > model = of_get_property(dt, "model", NULL);
> > > - if (model == NULL)
> > > + if (model == NULL) {
> > > + of_node_put(dt);
> > > return -ENODEV;
> > > + }
> > > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> > > strncmp(model, "iBook", strlen("iBook")) != 0 &&
> > > strcmp(model, "PowerMac7,2") != 0 &&
> > >
> >
> > is there any rule that says when to use strncmp ? it seems perfecly valid to use strcpy here
> > (what is done in the last cmp).
>
> Perhaps there are some characters after eg PowerBook that one doesn't want
> to compare with?
It seems to me that model has no '\0' in the end. If model is got from
the hardware then we should double check it - maybe harware is buggy.
Otherwise we'll overflow model.
But why strcmp(model, "PowerMac7,2")? IMO it should be replaced
with strncmp().
--
Vasiliy
On Tue, Aug 31, 2010 at 10:16 AM, Vasiliy Kulikov <[email protected]> wrote:
> On Tue, Aug 31, 2010 at 18:08 +0200, Julia Lawall wrote:
>> On Tue, 31 Aug 2010, walter harms wrote:
>> > > ? if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
>> > > ? ? ? strncmp(model, "iBook", strlen("iBook")) != 0 &&
>> > > ? ? ? strcmp(model, "PowerMac7,2") != 0 &&
>> > >
>> >
>> > is there any rule that says when to use strncmp ? it seems perfecly valid to use strcpy here
>> > (what is done in the last cmp).
>>
>> Perhaps there are some characters after eg PowerBook that one doesn't want
>> to compare with?
>
> It seems to me that model has no '\0' in the end. If model is got from
> the hardware then we should double check it - maybe harware is buggy.
> Otherwise we'll overflow model.
Model does have \0 at the end. This code is using strncmp to
purposefully ignore the model suffix.
> But why strcmp(model, "PowerMac7,2")? IMO it should be replaced
> with strncmp().
We use strcmp when parsing the device tree because the the length of
the model property string is unknown and in most cases we *must* match
the exact entire string, such as with this PowerMac7,2 example. Using
strncmp would also happen to match with something like
"PowerMac7,2345" which is not the desired behaviour.
g.
On Aug 29, 2010, at 4:52 AM, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r exists@
> local idexpression x;
> expression E,E1;
> 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
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
> ... when != of_node_put(x)
> when != if (...) { ... of_node_put(x); ... }
> (
> return <+...x...+>;
> |
> * return ...;
> )
> }
> ...>
> of_node_put(x);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> arch/powerpc/sysdev/qe_lib/qe.c | 1 +
> 1 file changed, 1 insertion(+)
applied to merge
- k