The mtd partition parser returns an allocated pointer array of
mtd_partition. The caller must free it. The array is used only for
add_mtd_partitions(), so free it just after the call.
Signed-off-by: Atsushi Nemoto <[email protected]>
---
drivers/mtd/maps/physmap.c | 17 ++++++++---------
1 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
index 7ca048d..cc26b41 100644
--- a/drivers/mtd/maps/physmap.c
+++ b/drivers/mtd/maps/physmap.c
@@ -29,7 +29,6 @@ struct physmap_flash_info {
struct map_info map[MAX_RESOURCES];
#ifdef CONFIG_MTD_PARTITIONS
int nr_parts;
- struct mtd_partition *parts;
#endif
};
@@ -56,14 +55,10 @@ static int physmap_flash_remove(struct platform_device *dev)
for (i = 0; i < MAX_RESOURCES; i++) {
if (info->mtd[i] != NULL) {
#ifdef CONFIG_MTD_PARTITIONS
- if (info->nr_parts) {
+ if (info->nr_parts || physmap_data->nr_parts)
del_mtd_partitions(info->mtd[i]);
- kfree(info->parts);
- } else if (physmap_data->nr_parts) {
- del_mtd_partitions(info->mtd[i]);
- } else {
+ else
del_mtd_device(info->mtd[i]);
- }
#else
del_mtd_device(info->mtd[i]);
#endif
@@ -86,6 +81,9 @@ static int physmap_flash_probe(struct platform_device *dev)
int err = 0;
int i;
int devices_found = 0;
+#ifdef CONFIG_MTD_PARTITIONS
+ struct mtd_partition *parts;
+#endif
physmap_data = dev->dev.platform_data;
if (physmap_data == NULL)
@@ -166,9 +164,10 @@ static int physmap_flash_probe(struct platform_device *dev)
goto err_out;
#ifdef CONFIG_MTD_PARTITIONS
- err = parse_mtd_partitions(info->cmtd, part_probe_types, &info->parts, 0);
+ err = parse_mtd_partitions(info->cmtd, part_probe_types, &parts, 0);
if (err > 0) {
- add_mtd_partitions(info->cmtd, info->parts, err);
+ add_mtd_partitions(info->cmtd, parts, err);
+ kfree(parts);
return 0;
}
--
1.5.6.3
On Wed, 12 Nov 2008 23:57:33 +0900 (JST), Atsushi Nemoto <[email protected]> wrote:
> The mtd partition parser returns an allocated pointer array of
> mtd_partition. The caller must free it. The array is used only for
> add_mtd_partitions(), so free it just after the call.
Note: all parsers except for the ar7part return kmalloced memory.
The ar7part parser returns a pointer of static array. While currently
there is no in-kernel user of this parser, there should not be cause
regression.
Anyway, I suppose ar7part parser also should return kmalloced memory,
as like as all other parsers.
---
Atsushi Nemoto
On Wed, 12 Nov 2008 23:57:33 +0900 (JST), Atsushi Nemoto <[email protected]> wrote:
> The mtd partition parser returns an allocated pointer array of
> mtd_partition. The caller must free it. The array is used only for
> add_mtd_partitions(), so free it just after the call.
>
> Signed-off-by: Atsushi Nemoto <[email protected]>
> ---
> drivers/mtd/maps/physmap.c | 17 ++++++++---------
> 1 files changed, 8 insertions(+), 9 deletions(-)
And many other callers of parse_mtd_partitions() also do not free the
returned memory. With a quick look, 21 of 36 have same defect.
drivers/mtd/devices/m25p80.c
drivers/mtd/devices/mtd_dataflash.c
drivers/mtd/maps/bfin-async-flash.c
drivers/mtd/maps/edb7312.c
drivers/mtd/maps/impa7.c
drivers/mtd/maps/intel_vr_nor.c
drivers/mtd/maps/solutionengine.c
drivers/mtd/nand/atmel_nand.c
drivers/mtd/nand/cafe_nand.c
drivers/mtd/nand/cmx270_nand.c
drivers/mtd/nand/cs553x_nand.c
drivers/mtd/nand/edb7312.c
drivers/mtd/nand/fsl_elbc_nand.c
drivers/mtd/nand/fsl_upm.c
drivers/mtd/nand/mxc_nand.c
drivers/mtd/nand/orion_nand.c
drivers/mtd/nand/ppchameleonevb.c
drivers/mtd/nand/sharpsl.c
drivers/mtd/nand/tmio_nand.c
drivers/mtd/nand/ts7250.c
drivers/mtd/onenand/generic.c
Volunteers? ;)
---
Atsushi Nemoto
On Wed, Nov 12, 2008 at 10:55, Atsushi Nemoto wrote:
> On Wed, 12 Nov 2008 23:57:33 +0900 (JST), Atsushi Nemoto wrote:
>> The mtd partition parser returns an allocated pointer array of
>> mtd_partition. The caller must free it. The array is used only for
>> add_mtd_partitions(), so free it just after the call.
>>
>> Signed-off-by: Atsushi Nemoto <[email protected]>
>> ---
>> drivers/mtd/maps/physmap.c | 17 ++++++++---------
>> 1 files changed, 8 insertions(+), 9 deletions(-)
>
> And many other callers of parse_mtd_partitions() also do not free the
> returned memory. With a quick look, 21 of 36 have same defect.
i wonder why we duplicate this same code block in so many places. and
why does every driver have to declare its own list of parsers ? cant
we unify all of these in one place ?
> drivers/mtd/maps/bfin-async-flash.c
i can fix this one if no one else gets to it
-mikex
On Wed, 2008-11-12 at 11:08 -0500, Mike Frysinger wrote:
> i wonder why we duplicate this same code block in so many places. and
> why does every driver have to declare its own list of parsers ? cant
> we unify all of these in one place ?
Well, most boards only really want _one_ partition type; maybe two. And
the probes can be quite expensive and have false positives, so we don't
want to be doing all the probes for all devices.
It might make sense to just pass a bitmask indicating which partition
probes to use. Assuming we can agree on an ordering, that is :)
Another complication is that some boards have registered the whole
device first, while others have just registered the partitions.
I suspect it might be better to take the simple bug-fix approach for
now, given that we are planning a revamp of the MTD core API anyway --
we can redo partitions properly at the point, so they aren't just an
evil hack.
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation
On Tue, 2008-11-18 at 13:57 +0000, David Woodhouse wrote:
> I suspect it might be better to take the simple bug-fix approach for
> now, given that we are planning a revamp of the MTD core API anyway --
> we can redo partitions properly at the point, so they aren't just an
> evil hack.
Off topic: is this going to really happen? Are you have real plans
to do this?
Also, you also mentioned that you have sysfs for MTD, are we going to
see them in mtd-2.6.git any time soon?
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)
On Tue, 2008-11-18 at 16:04 +0200, Artem Bityutskiy wrote:
> Off topic: is this going to really happen? Are you have real plans
> to do this?
The plans for partitioning aren't entirely defined, but yes -- we have
to do it.
> Also, you also mentioned that you have sysfs for MTD, are we going to
> see them in mtd-2.6.git any time soon?
I did a bunch of it when I was on the plane to the kernel summit and
plumbers conference. I should get it to actually compile and then put it
in a separate tree -- I don't think it'll be ready for merging to Linus
imminently.
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation