Command line parsing
Is there any particular reason parse_cmdline_partitions() in drivers/mtd/cmdlineparse.c doesn't completely setup the partition table(s) it builds? It fails to initialize the offsets and sizes of any partition tabled unless it is the one named by "master". This one has the offsets and lengths filled in but the others are left zero with the exception of partitions specified with "-" or SIZE_REMAINING. I would have expected all partitions described in the cmdline to be initialized if any are.
Dave Braun
Quoted 9 lines · David A. Braun
Is there any particular reason
parse_cmdline_partitions()indrivers/mtd/cmdlineparse.cdoesn't completely setup the partition table(s) it builds? It fails to initialize the offsets and sizes of any partition tabled unless it is the one named by "master". This one has the offsets and lengths filled in but the others are left zero with the exception of partitions specified with "-" orSIZE_REMAINING. I would have expected all partitions described in the cmdline to be initialized if any are.
Iirc, the code was written by someone at Sysgo, possibly Robert, and hasn't been touched for quite a while. So if you have any patches that improve the situation for you, feel free to send them.
Jörn
Signature · 4 lines
Rules of Optimization: Rule 1: Don't do it. Rule 2 (for experts only): Don't do it yet. -- M.A. Jackson
Quoted 11 lines · Jörn Engel
Quoted 9 lines · David A. Braun
Is there any particular reason
parse_cmdline_partitions()indrivers/mtd/cmdlineparse.cdoesn't completely setup the partition table(s) it builds? It fails to initialize the offsets and sizes of any partition tabled unless it is the one named by "master". This one has the offsets and lengths filled in but the others are left zero with the exception of partitions specified with "-" orSIZE_REMAINING. I would have expected all partitions described in the cmdline to be initialized if any are.
Quoted 3 lines
Iirc, the code was written by someone at Sysgo, possibly Robert, and hasn't been touched for quite a while. So if you have any patches that improve the situation for you, feel free to send them.
Acutally it was me :-), but someone decided to remove my name and just leave in the reference to our company... Anyway you're right about my not having touched that code. Looking at it briefly, though, I don't really spot anything suspicous. Apart from the (potential) oddity that the cmdline is saved and processed much later at a time the flash hardware is actually, I don't see what is not initialized.
In other words, David, what is the actual problem you're seeing?
Regards,
Marius
Signature · 4 lines
Marius Groeger <[email protected]> SYSGO AG Embedded and Real-Time Software Voice: +49 6136 9948 0 FAX: +49 6136 9948 10 www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com
Quoted 16 lines · Marius Groeger
Quoted 11 lines · Jörn Engel
Quoted 9 lines · David A. Braun
Is there any particular reason
parse_cmdline_partitions()indrivers/mtd/cmdlineparse.cdoesn't completely setup the partition table(s) it builds? It fails to initialize the offsets and sizes of any partition tabled unless it is the one named by "master". This one has the offsets and lengths filled in but the others are left zero with the exception of partitions specified with "-" orSIZE_REMAINING. I would have expected all partitions described in the cmdline to be initialized if any are.Quoted 3 lines
Iirc, the code was written by someone at Sysgo, possibly Robert, and hasn't been touched for quite a while. So if you have any patches that improve the situation for you, feel free to send them.
Quoted 6 lines
Acutally it was me :-), but someone decided to remove my name and just leave in the reference to our company... Anyway you're right about my not having touched that code. Looking at it briefly, though, I don't really spot anything suspicous. Apart from the (potential) oddity that the cmdline is saved and processed much later at a time the flash hardware is actually, I don't see what is not initialized.
[ slightly off-topic ]
If you want your copyright line restored and don't have cvs access, send me a patch.
For those less aware of how copyright law works:
The copyright itself is independent of some lines explicitly claiming it for somebody. So this is not about a legal issue, just politeness.
Jörn
Signature · 3 lines
I can say that I spend most of my time fixing bugs even if I have lots of new features to implement in mind, but I give bugs more priority. -- Andrea Arcangeli, 2000
Quoted 1 line · Marius Groeger
in the reference to our company... Anyway you're right about my not having touched that code.
Add "... in a long time" to make this more reasonable to parse :-)
And sorry for the noise...
Marius
Signature · 4 lines
Marius Groeger <[email protected]> SYSGO AG Embedded and Real-Time Software Voice: +49 6136 9948 0 FAX: +49 6136 9948 10 www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com
If the name in master->name doesn't match any of the partitions mentioned on the kernel command line then *pparts->offset is not initialized. The result is that all the partitions start at 0. Also the length of the "SIZE_REMAINING" partition is never initialized. Also the return value is -EINVAL instead of the number of partitions. This results in none of the partitions being pass to the driver. The only partition that appears in /proc/partitions is one for the whole device.
There are two problems. The if statement
if ((!mtd_id) || (!strcmp(part->mtd_id, mtd_id)))
{
...before the innermost for loop prevents the non-matching cmdline partitions from being initialized. Secondly the return statement just after this same for loop stops processing any remaining partitions following the matching partition.
As it turns out after looking over the code some more the table in question is internal to the cmdline parser anyway so no harm is done. Any partition declared on the cmdline that is referenced by a mtd driver gets initialized correctly. Unreferenced ones don't and need not be. There is however a small amount of kernel memory wasted on the partition table in cmdlineparts. It doesn't look like it ever gets released, not even if the cmdline parser gets de-registered (which it doesn't).
dave
Quoted 41 lines · Jörn Engel
Quoted 16 lines · Marius Groeger
Quoted 13 lines · Jörn Engel
Quoted 10 lines · David A. Braun
Is there any particular reason
parse_cmdline_partitions()indrivers/mtd/cmdlineparse.cdoesn't completely setup the partition table(s) it builds? It fails to initialize the offsets and sizes of any partition tabled unless it is the one named by "master". This one has the offsets and lengths filled in but the others are left zero with the exception of partitions specified with "-" orSIZE_REMAINING. I would have expected all partitions described in the cmdline to be initialized if any are.Quoted 8 lines
Quoted 5 lines
Iirc, the code was written by someone at Sysgo, possibly Robert, and hasn't been touched for quite a while. So if you have any patches that improve the situation for you, feel free to send them.
Acutally it was me :-), but someone decided to remove my name and just leave in the reference to our company... Anyway you're right about my not having touched that code. Looking at it briefly, though, I don't really spot anything suspicous. Apart from the (potential) oddity that the cmdline is saved and processed much later at a time the flash hardware is actually, I don't see what is not initialized.
[ slightly off-topic ]
If you want your copyright line restored and don't have cvs access, send me a patch.
For those less aware of how copyright law works:
The copyright itself is independent of some lines explicitly claiming it for somebody. So this is not about a legal issue, just politeness.Jörn
Quoted 1 line · David A. Braun
If the name in
master->namedoesn't match any of the partitions mentioned on the kernel command line then*pparts->offsetis not initialized. The result is that all the partitions start at 0. Also the length of the "SIZE_REMAINING" partition is never initialized. Also the return value is-EINVALinstead of the number of partitions. This results in none of the partitions being pass to the driver. The only partition that appears in/proc/partitionsis one for the whole device.
Thanks for digging so deeply in to the code. I suggest it would be best if you prepare a patch and send it to the list.
Regards,
Marius
Signature · 4 lines
Marius Groeger <[email protected]> SYSGO AG Embedded and Real-Time Software Voice: +49 6136 9948 0 FAX: +49 6136 9948 10 www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com
Quoted 2 lines · David A. Braun
If the name in
master->namedoesn't match any of the partitions mentioned on the kernel command line then*pparts->offsetis not initialized. The result is that all the partitions start at 0. Also the length of the "SIZE_REMAINING" partition is never initialized. Also the return value is-EINVALinstead of the number of partitions. This results in none of the partitions being pass to the driver. The only partition that appears in/proc/partitionsis one for the whole device.
Is not it intentional?
There is a comment:
<Q> mtd-id := unique name used in mapping driver/device (mtd->name) </Q>
These names are identifing the separate devices rather than partitions. And IMO it's only way to deal multiple flash devices with different partitioning.
--
Regards.
Tim.
as Clinton said - it depends on what you mean by "it"
If by "it" you mean the return value of EINVAL when the named partition by master->name is not found then you are correct. My mistake.
dave
Quoted 17 lines · Timofei V. Bondarenko
Quoted 2 lines · David A. Braun
If the name in
master->namedoesn't match any of the partitions mentioned on the kernel command line then*pparts->offsetis not initialized. The result is that all the partitions start at 0. Also the length of the "SIZE_REMAINING" partition is never initialized. Also the return value is-EINVALinstead of the number of partitions. This results in none of the partitions being pass to the driver. The only partition that appears in/proc/partitionsis one for the whole device.Is not it intentional?
There is a comment:
<Q> mtd-id := unique name used in mapping driver/device (mtd->name) </Q>These names are identifing the separate devices rather than partitions. And IMO it's only way to deal multiple flash devices with different partitioning.
Signature · 2 lines
Regards. Tim.