On Mon, 02 Jan 2012 04:06:39 +0100 Carl-Daniel Hailfinger <[email protected]> wrote:
> Layout file reading should happen after option parsing like all other > file accesses. > Guard against multiple --layout parameters. > > Side note: This fixes an inconsistency which impacts the log file patch. > > Signed-off-by: Carl-Daniel Hailfinger <[email protected]> > > Index: flashrom-postpone_layoutfile_reading/cli_classic.c > =================================================================== > --- flashrom-postpone_layoutfile_reading/cli_classic.c (Revision 1482) > +++ flashrom-postpone_layoutfile_reading/cli_classic.c (Arbeitskopie) > @@ -204,6 +204,7 @@ > }; > > char *filename = NULL; > + char *layoutfile = NULL; > char *tempstr = NULL; > char *pparam = NULL; > > @@ -290,9 +291,12 @@ > force = 1; > break; > case 'l': > - tempstr = strdup(optarg); > - if (read_romlayout(tempstr)) > + 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. i investigated if we can distinguish between short and long options to display a correct error message (list the option actually supplied instead of a hardcoded string). the good news: it is possible actually. the bad news: it is more complicated than technically needed, doable, but probably not worth it. so of course ill try it in a few :) > cli_classic_abort_usage(); > + } > + layoutfile = strdup(optarg); > break; > case 'i': > tempstr = strdup(optarg); > @@ -390,9 +394,6 @@ > cli_classic_abort_usage(); > } > > - if (process_include_args()) > - cli_classic_abort_usage(); > - > /* FIXME: Print the actions flashrom will take. */ > > if (list_supported) { > @@ -407,6 +408,11 @@ > } > #endif > > + if (layoutfile && read_romlayout(layoutfile)) > + cli_classic_abort_usage(); > + if (process_include_args()) > + cli_classic_abort_usage(); > + 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). Acked-by: Stefan Tauner <[email protected]> PS: you may wanna remove the -m short option with this one already. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
