On Wed, 11 Jan 2012 03:13:48 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 05.01.2012 03:26 schrieb Stefan Tauner:
On Mon, 02 Jan 2012 04:06:39 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
if (layoutfile) {fprintf(stderr, "Error: --layout specified ""more than once. Aborting.\n");supporting more than one layout file might become handy for some. for example when using the same hardware platform with multiple firmwares one might have a common file which specifies for example the boot block and multiple other files for the different firmwares respectively. far-fetched? probably. hard to implement? not with an easy and ready to use data structure like an innovative linked list! ;)
i am not suggesting adding this now. it just sprang to my mind when thinking about this.
The usage of multiple layout files in one flashrom invocation is a usability nightmare. If there are any conflicting region names or any conflicting region definitions, which one has precedence?
with all due respect, this is bullshit :) here is why: currently there is no check for multiple equally named regions in a single file either! is precedence documented anywhere? i guess not. :) the parser in read_romlayout() just does not care. precedence is defined by the search algorithm in find_romentry() afaics.
if we check in the parser if an equally named region already exists and bail out in that case, i dont see a problem at all. this is needed anyway due to the "inconsistency" mentioned above.
side note: layout file parsing should be postponed after argument processing too. if one specifies -l <file> before -V+ there are no debug messages from the layout file parser....
btw: what is a "region definition"? do you mean overlapping address ranges? this is not handled yet either iirc. it does not make sense either. one might one to include existing PR* regions, FREG regions and maybe even other things that overlap multiple times... as long as there are no images/data sources associated with overlapping and *selected* regions, there is no need for precedence. this would have to be dealt with in the "layout: Add -i <image>[:<file>] support" patch.
And we will get mails with complaints like "why doesn't specifying multiple layout files work for me".
if we ever meet, ill buy you a beer for any such (legit) mail. :)
Nack from me. If a user does not have the layout file he/she wants, he/she can edit an existing one with a text editor and/or write a new one. Users who don't understand layout files should be protected from shooting themselves with random layout file combinations.
users that dont understand layout files and using multiple ones anyway? i dont want to save such users TBH, else i would have to remove all flashrom options other than -R (see today's emails, why even -V can apparently lead to a bricked system for some users...). flashrom is a mighty tool, having a safety net or two is very important and i dont want to remove any, but bringing this argument up here is not fair imho.
side note: we do not (i.e. i did not) check for duplicate -i arguments: Looking for region "pr0"... found. Looking for region "pr0"... found. Using regions: "pr0", "pr0".
not a big issue i hope (i did not check it but presume it to be cosmetic only).
Argh. This is not really a bug (it can't cause problems AFAICS), but for UI consistency reasons we should indeed check for duplicate -i arguments and abort flashrom if we find some.
added to my todo list... not on top though, so if someone beats me to it, i am happy to ack/review too.