Change in ...flashrom[master]: [RFC] cli_classic: Add `--mode (read|write|verify|erase)` parameter - flashrom-gerrit

17 Jan 2019

Nico Huber has uploaded this change for review. (
https://review.coreboot.org/c/flashrom/+/30979


Change subject: [RFC] cli_classic: Add `--mode (read|write|verify|erase)` parameter
......................................................................

[RFC] cli_classic: Add `--mode (read|write|verify|erase)` parameter

In case the user only wants to specify files for specific layout
regions, we need a way to set the operation without specifying a
file. Instead of making the <filename> argument optional, we can
add a new syntax for this particular purpose. This way, we avoid
complex command line parsing and can do more sanity checks and
provide better error messages.

TODO: Update manpage in case this gets accepted.

Change-Id: Idfba11ec9991aac423b07f68f7dc45e7bebbb06b
Signed-off-by: Nico Huber <nico.huber(a)secunet.com&gt;
---
M cli_classic.c
1 file changed, 42 insertions(+), 2 deletions(-)



  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/30979/1

diff --git a/cli_classic.c b/cli_classic.c
index 324e145..d09a8d1 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -39,7 +39,8 @@
 	       "-z|"
 #endif
 	       "-p <programmername>[:<parameters>] [-c
<chipname>]\n"
-	       "[-E|(-r|-w|-v) <file>] [(-l <layoutfile>|--ifd) [-i
<imagename>]...] [-n] [-N] [-f]]\n"
+	       "[(-r|-w|-v|-E) <file>|-m (r|w|v|e)] [(-l <layoutfile>|--ifd)
[-i <region>]...]\n"
+	       "[-n] [-N] [-f]]\n"
 	       "[-V[V[V]]] [-o <logfile>]\n\n", name);
 
 	printf(" -h | --help                        print this help text\n"
@@ -47,6 +48,8 @@
 	       " -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"
+	       " -m | --mode <operation>            for use with --include
<region>:<file>,\n"
+	       "                                    specify the mode of operation
(r|w|v|e)\n"
 	       " -E | --erase                       erase flash memory\n"
 	       " -V | --verbose                     more verbose output\n"
 	       " -c | --chip <chipname>             probe only for specified flash
chip\n"
@@ -123,6 +126,7 @@
 		{"write",		1, NULL, 'w'},
 		{"erase",		0, NULL, 'E'},
 		{"verify",		1, NULL, 'v'},
+		{"mode",		1, NULL, 'm'},
 		{"noverify",		0, NULL, 'n'},
 		{"noverify-all",	0, NULL, 'N'},
 		{"chip",		1, NULL, 'c'},
@@ -200,6 +204,25 @@
 			filename = strdup(optarg);
 			verify_it = 1;
 			break;
+		case 'm':
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation specified. Aborting.\n");
+				cli_classic_abort_usage();
+			}
+			/* check that `optarg` is a prefix of a known operation */
+			if (strncmp(optarg, "read", strlen(optarg)) == 0) {
+				read_it = 1;
+			} else if (strncmp(optarg, "write", strlen(optarg)) == 0) {
+				write_it = 1;
+			} else if (strncmp(optarg, "verify", strlen(optarg)) == 0) {
+				verify_it = 1;
+			} else if (strncmp(optarg, "erase", strlen(optarg)) == 0) {
+				erase_it = 1;
+			} else {
+				fprintf(stderr, "Unknown operation specified: '%s'\n", optarg);
+				cli_classic_abort_usage();
+			}
+			break;
 		case 'n':
 			if (verify_it) {
 				fprintf(stderr, "--verify and --noverify are mutually exclusive.
Aborting.\n");
@@ -404,7 +427,7 @@
 		cli_classic_abort_usage();
 	}
 
-	if ((read_it | write_it | verify_it) && check_filename(filename,
"image")) {
+	if (filename && check_filename(filename, "image")) {
 		cli_classic_abort_usage();
 	}
 	if (layoutfile && check_filename(layoutfile, "layout")) {
@@ -654,6 +677,23 @@
 	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !dont_verify_it);
 	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, !dont_verify_all);
 
+	if (!filename && (read_it | write_it | verify_it)) {
+		if (layout->num_entries == 0) {
+			msg_gerr("Error: The specified operation requires a file.\n");
+			ret = 1;
+			goto out_shutdown;
+		}
+		for (i = 0; i < layout->num_entries; i++) {
+			if (!layout->entries[i].file) {
+				msg_gerr("Error: Region \"%s\" requires a file argument.\n",
+					 layout->entries[i].name);
+				ret = 1;
+			}
+			if (ret)
+				goto out_shutdown;
+		}
+	}
+
 	/* FIXME: We should issue an unconditional chip reset here. This can be
 	 * done once we have a .reset function in struct flashchip.
 	 * Give the chip time to settle.

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/30979
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Idfba11ec9991aac423b07f68f7dc45e7bebbb06b
Gerrit-Change-Number: 30979
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de&gt;
Gerrit-MessageType: newchange



    