On Fri, Apr 17, 2009 at 11:35 PM, Grant Likely <grant.lik...@secretlab.ca> wrote: > On Fri, Apr 17, 2009 at 4:07 PM, Ilpo Järvinen >> [PATCH] powerpc & microblaze: add missing of_node_put to error handling >> >> While reviewing some microblaze patches a while ago, I noticed >> a suspicious error handling in of_irq_map_one(), which turned >> out to be a copy from arch/powerpc. Grant Likely >> <grant.lik...@secretlab.ca> confirmed that this is a real bug. >> >> Merge error handling paths using goto with the normal return >> path. >> >> Powerppc compile tested. >> >> Signed-off-by: Ilpo Järvinen <ilpo.jarvi...@helsinki.fi> > > Looks right to me (but I haven't tested). > > Acked-by: Grant Likely <grant.lik...@secretlab.ca>
I've picked up the powerpc half of this patch, tested it, and pushed it out to my -next tree and asked Ben or Paul to pull. Michal, I leave it to you to pick up the Microblaze half. Cheers, g. > >> --- >> arch/microblaze/kernel/prom_parse.c | 11 +++++------ >> arch/powerpc/kernel/prom_parse.c | 11 +++++------ >> 2 files changed, 10 insertions(+), 12 deletions(-) >> >> diff --git a/arch/microblaze/kernel/prom_parse.c >> b/arch/microblaze/kernel/prom_parse.c >> index ae0352e..d16c32f 100644 >> --- a/arch/microblaze/kernel/prom_parse.c >> +++ b/arch/microblaze/kernel/prom_parse.c >> @@ -903,7 +903,7 @@ int of_irq_map_one(struct device_node *device, >> struct device_node *p; >> const u32 *intspec, *tmp, *addr; >> u32 intsize, intlen; >> - int res; >> + int res = -EINVAL; >> >> pr_debug("of_irq_map_one: dev=%s, index=%d\n", >> device->full_name, index); >> @@ -926,21 +926,20 @@ int of_irq_map_one(struct device_node *device, >> >> /* Get size of interrupt specifier */ >> tmp = of_get_property(p, "#interrupt-cells", NULL); >> - if (tmp == NULL) { >> - of_node_put(p); >> - return -EINVAL; >> - } >> + if (tmp == NULL) >> + goto out; >> intsize = *tmp; >> >> pr_debug(" intsize=%d intlen=%d\n", intsize, intlen); >> >> /* Check index */ >> if ((index + 1) * intsize > intlen) >> - return -EINVAL; >> + goto out; >> >> /* Get new specifier and map it */ >> res = of_irq_map_raw(p, intspec + index * intsize, intsize, >> addr, out_irq); >> +out: >> of_node_put(p); >> return res; >> } >> diff --git a/arch/powerpc/kernel/prom_parse.c >> b/arch/powerpc/kernel/prom_parse.c >> index 8f0856f..8362620 100644 >> --- a/arch/powerpc/kernel/prom_parse.c >> +++ b/arch/powerpc/kernel/prom_parse.c >> @@ -971,7 +971,7 @@ int of_irq_map_one(struct device_node *device, int >> index, struct of_irq *out_irq >> struct device_node *p; >> const u32 *intspec, *tmp, *addr; >> u32 intsize, intlen; >> - int res; >> + int res = -EINVAL; >> >> DBG("of_irq_map_one: dev=%s, index=%d\n", device->full_name, index); >> >> @@ -995,21 +995,20 @@ int of_irq_map_one(struct device_node *device, int >> index, struct of_irq *out_irq >> >> /* Get size of interrupt specifier */ >> tmp = of_get_property(p, "#interrupt-cells", NULL); >> - if (tmp == NULL) { >> - of_node_put(p); >> - return -EINVAL; >> - } >> + if (tmp == NULL) >> + goto out; >> intsize = *tmp; >> >> DBG(" intsize=%d intlen=%d\n", intsize, intlen); >> >> /* Check index */ >> if ((index + 1) * intsize > intlen) >> - return -EINVAL; >> + goto out; >> >> /* Get new specifier and map it */ >> res = of_irq_map_raw(p, intspec + index * intsize, intsize, >> addr, out_irq); >> +out: >> of_node_put(p); >> return res; >> } >> -- >> 1.5.6.5 >> > > > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev