Daniel Campello has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52362 )
Change subject: cli_classic: Make -r/-w/-v argument optional ......................................................................
cli_classic: Make -r/-w/-v argument optional
Makes filename optional from -r/-w/-v since the -i parameter allows building the image to be written from multiple files, and regions to be read from flash and written to separate image files,
Signed-off-by: Daniel Campello campello@chromium.org Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41 --- M cli_classic.c M flashrom.c 2 files changed, 79 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/52362/1
diff --git a/cli_classic.c b/cli_classic.c index d34b8b3..f86b2bd 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -46,9 +46,9 @@
printf(" -h | --help print this help text\n" " -R | --version print version (release)\n" - " -r | --read <file> read flash and save to <file>\n" - " -w | --write <file> write <file> to flash\n" - " -v | --verify <file> verify flash against <file>\n" + " -r | --read [<file>] read flash and save to <file>\n" + " -w | --write [<file>] write <file> to flash\n" + " -v | --verify [<file>] verify flash against <file>\n" " -E | --erase erase flash memory\n" " -V | --verbose more verbose output\n" " -c | --chip <chipname> probe only for specified flash chip\n" @@ -137,6 +137,18 @@ return 0; }
+static char *get_optional_filename(char *argv[]) +{ + char *filename = NULL; + + if (optarg == NULL && argv[optind] != NULL && argv[optind][0] != '-') + filename = strdup(argv[optind++]); + else if (optarg != NULL) + filename = strdup(optarg); + + return filename; +} + int main(int argc, char *argv[]) { const struct flashchip *chip = NULL; @@ -173,12 +185,12 @@ int ret = 0; unsigned int wp_start = 0, wp_len = 0;
- static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:"; + static const char optstring[] = "r::Rw::v::nNVEfc:l:i:p:Lzho:"; static const struct option long_options[] = { - {"read", 1, NULL, 'r'}, - {"write", 1, NULL, 'w'}, + {"read", 2, NULL, 'r'}, + {"write", 2, NULL, 'w'}, {"erase", 0, NULL, 'E'}, - {"verify", 1, NULL, 'v'}, + {"verify", 2, NULL, 'v'}, {"noverify", 0, NULL, 'n'}, {"noverify-all", 0, NULL, 'N'}, {"chip", 1, NULL, 'c'}, @@ -237,12 +249,12 @@ switch (opt) { case 'r': cli_classic_validate_singleop(&operation_specified); - filename = strdup(optarg); + filename = get_optional_filename(argv); read_it = 1; break; case 'w': cli_classic_validate_singleop(&operation_specified); - filename = strdup(optarg); + filename = get_optional_filename(argv); write_it = 1; break; case 'v': @@ -251,7 +263,7 @@ if (dont_verify_it) { cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n"); } - filename = strdup(optarg); + filename = get_optional_filename(argv); verify_it = 1; break; case 'n': @@ -444,7 +456,7 @@
if (optind < argc) cli_classic_abort_usage("Error: Extra parameter found.\n"); - if ((read_it | write_it | verify_it) && check_filename(filename, "image")) + if ((read_it | write_it | verify_it) && (!filename || check_filename(filename, "image"))) cli_classic_abort_usage(NULL); if (layoutfile && check_filename(layoutfile, "layout")) cli_classic_abort_usage(NULL); @@ -643,6 +655,52 @@ goto out_shutdown; }
+ /* + * Common rules for -r/-w/-v syntax parsing: + * - If no filename is specified at all, quit. + * - If no filename is specified for -r/-w/-v, but files are specified + * for -i, then the number of file arguments for -i options must be + * equal to the total number of -i options. + * + * Rules for reading: + * - If files are specified for -i args but not -r, do partial reads for + * each -i arg, creating a new file for each region. Each -i option + * must specify a filename. + * - If filenames are specified for -r and -i args, then: + * - Do partial read for each -i arg, creating a new file for + * each region where a filename is provided (-i region:filename). + * - Create a ROM-sized file with partially filled content. For each + * -i arg, fill the corresponding offset with content from ROM. + * + * Rules for writing and verifying: + * - If files are specified for both -w/-v and -i args, -i files take + * priority. + * - If file is specified for -w/-v and no files are specified with -i + * args, then the file is to be used for writing/verifying the entire + * ROM. + * - If files are specified for -i args but not -w, do partial writes + * for each -i arg. Likewise for -v and -i args. All -i args must + * supply a filename. Any omission is considered ambiguous. + * - Regions with a filename associated must not overlap. This is also + * considered ambiguous. Note: This is checked later since it requires + * processing the layout/fmap first. + */ + if ((read_it | write_it | verify_it) && !filename) { + struct layout_include_args *arg; + if (!include_args) { + msg_gerr("Error: No image file specified.\n"); + ret = 1; + goto out_shutdown; + } + + for (arg = include_args; arg; arg = arg->next) { + if (check_filename(arg->file, "region")) { + ret = 1; + goto out_shutdown; + } + } + } + if (set_wp_enable && set_wp_disable) { msg_ginfo("Error: --wp-enable and --wp-disable are mutually exclusive\n"); ret = 1; diff --git a/flashrom.c b/flashrom.c index c695f53..5b52491 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1537,7 +1537,8 @@ goto out_free; }
- ret = write_buf_to_file(buf, size, filename); + if (filename) + ret = write_buf_to_file(buf, size, filename); out_free: free(buf); msg_cinfo("%s.\n", ret ? "FAILED" : "done"); @@ -2727,8 +2728,10 @@ }
/* Read '-w' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; + if (filename) { + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + } /* * ... then update newcontents with contents from files provided to '-i' * args if needed. @@ -2761,8 +2764,10 @@ }
/* Read '-v' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; + if (filename) { + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + } /* * ... then update newcontents with contents from files provided to '-i' * args if needed.
-
Daniel Campello (Code Review)