Fix assorted documentation, frontend and printing bugs

Change the command line interface to make file names positional.
Add more sanity checks to the command line parser.

Corresponding to flashrom svn r998.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Michael Karcher <flashrom@mkarcher.dialup.fu-berlin.de>

1 files changed, 130 insertions, 72 deletions

diff --git a/cli_classic.c b/cli_classic.c
index 24762fe..6e8f098 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -37,33 +37,53 @@ void cli_classic_usage(const char *name)
 	int remaining = 0;
 	enum programmer p;
 
-	printf("Usage: %s [-VfLzhR] [-E|-r file|-w file|-v file] [-c chipname]\n"
-              "       [-m [vendor:]part] [-l file] [-i image] [-p programmer]\n\n", name);
-
-	printf("Please note that the command line interface for flashrom will "
-		"change before\nflashrom 1.0. Do not use flashrom in scripts "
-		"or other automated tools without\nchecking that your flashrom"
-		" version won't interpret options in a different way.\n\n");
-
-	printf
-	    ("   -r | --read:                      read flash and save into file\n"
-	     "   -w | --write:                     write file into flash\n"
-	     "   -v | --verify:                    verify flash against file\n"
-	     "   -n | --noverify:                  don't verify flash against file\n"
-	     "   -E | --erase:                     erase flash device\n"
-	     "   -V | --verbose:                   more verbose output\n"
-	     "   -c | --chip <chipname>:           probe only for specified flash chip\n"
+	printf("Usage: %s [-n] [-V] [-f] [-h|-R|-L|"
+#if PRINT_WIKI_SUPPORT == 1
+	         "-z|"
+#endif
+	         "-E|-r <file>|-w <file>|-v <file>]\n"
+	       "       [-c <chipname>] [-m [<vendor>:]<part>] [-l <file>]\n"
+	       "       [-i <image>] [-p <programmername>[:<parameters>]]\n",
+	       name);
+
+	printf("Please note that the command line interface for flashrom has "
+	         "changed between\n"
+	       "0.9.1 and 0.9.2 and will change again before flashrom 1.0.\n"
+	       "Do not use flashrom in scripts or other automated tools "
+	         "without checking\n"
+	       "that your flashrom version won't interpret options in a "
+	         "different way.\n\n");
+
+	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"
+	       "   -E | --erase                      erase flash device\n"
+	       "   -V | --verbose                    more verbose output\n"
+	       "   -c | --chip <chipname>            probe only for specified "
+	         "flash chip\n"
 #if INTERNAL_SUPPORT == 1
-	     "   -m | --mainboard <[vendor:]part>: override mainboard settings\n"
+	       /* FIXME: --mainboard should be a programmer parameter */
+	       "   -m | --mainboard <[vendor:]part>  override mainboard "
+	         "detection\n"
 #endif
-	     "   -f | --force:                     force write without checking image\n"
-	     "   -l | --layout <file.layout>:      read ROM layout from file\n"
-	     "   -i | --image <name>:              only flash image name from flash layout\n"
-	     "   -L | --list-supported:            print supported devices\n"
+	       "   -f | --force                      force specific operations "
+	         "(see man page)\n"
+	       "   -n | --noverify                   don't auto-verify\n"
+	       "   -l | --layout <file>              read ROM layout from "
+	         "<file>\n"
+	       "   -i | --image <name>               only flash image <name> "
+	         "from flash layout\n"
+	       "   -L | --list-supported             print supported devices\n"
 #if PRINT_WIKI_SUPPORT == 1
-	     "   -z | --list-supported-wiki:       print supported devices in wiki syntax\n"
+	       "   -z | --list-supported-wiki        print supported devices "
+	         "in wiki syntax\n"
 #endif
-	     "   -p | --programmer <name>:         specify the programmer device");
+	       "   -p | --programmer <name>[:<param>] specify the programmer "
+	         "device");
 
 	for (p = 0; p < PROGRAMMER_INVALID; p++) {
 		pname = programmer_table[p].name;
@@ -88,12 +108,19 @@ void cli_classic_usage(const char *name)
 			printf(")\n");
 		}
 	}
-		
-	printf(
-	     "   -h | --help:                      print this help text\n"
-	     "   -R | --version:                   print the version (release)\n"
-	     "\nYou can specify one of -E, -r, -w, -v or no operation. If no operation is\n"
-	     "specified, then all that happens is that flash info is dumped.\n\n");
+
+	printf("\nYou can specify one of -h, -R, -L, "
+#if PRINT_WIKI_SUPPORT == 1
+	         "-z, "
+#endif
+	         "-E, -r, -w, -v or no operation.\n"
+	       "If no operation is specified, flashrom will only probe for "
+	         "flash chips.\n\n");
+}
+
+void cli_classic_abort_usage(const char *name)
+{
+	printf("Please run \"%s --help\" for usage info.\n", name);
 	exit(1);
 }
 
@@ -115,16 +142,12 @@ int cli_classic(int argc, char *argv[])
 	int operation_specified = 0;
 	int i;
 
-#if PRINT_WIKI_SUPPORT == 1
-	const char *optstring = "rRwvnVEfc:m:l:i:p:Lzh";
-#else
-	const char *optstring = "rRwvnVEfc:m:l:i:p:Lh";
-#endif
+	const char *optstring = "r:Rw:v:nVEfc:m:l:i:p:Lzh";
 	static struct option long_options[] = {
-		{"read", 0, 0, 'r'},
-		{"write", 0, 0, 'w'},
+		{"read", 1, 0, 'r'},
+		{"write", 1, 0, 'w'},
 		{"erase", 0, 0, 'E'},
-		{"verify", 0, 0, 'v'},
+		{"verify", 1, 0, 'v'},
 		{"noverify", 0, 0, 'n'},
 		{"chip", 1, 0, 'c'},
 		{"mainboard", 1, 0, 'm'},
@@ -133,9 +156,7 @@ int cli_classic(int argc, char *argv[])
 		{"layout", 1, 0, 'l'},
 		{"image", 1, 0, 'i'},
 		{"list-supported", 0, 0, 'L'},
-#if PRINT_WIKI_SUPPORT == 1
 		{"list-supported-wiki", 0, 0, 'z'},
-#endif
 		{"programmer", 1, 0, 'p'},
 		{"help", 0, 0, 'h'},
 		{"version", 0, 0, 'R'},
@@ -147,18 +168,15 @@ int cli_classic(int argc, char *argv[])
 	char *tempstr = NULL;
 
 	print_version();
-
-	if (argc > 1) {
-		/* Yes, print them. */
-		printf_debug("The arguments are:\n");
-		for (i = 1; i < argc; ++i)
-			printf_debug("%s\n", argv[i]);
-	}
+	print_banner();
 
 	if (selfcheck())
 		exit(1);
 
 	setbuf(stdout, NULL);
+	/* FIXME: Delay all operation_specified checks until after command
+	 * line parsing to allow --help overriding everything else.
+	 */
 	while ((opt = getopt_long(argc, argv, optstring,
 				  long_options, &option_index)) != EOF) {
 		switch (opt) {
@@ -166,16 +184,18 @@ int cli_classic(int argc, char *argv[])
 			if (++operation_specified > 1) {
 				fprintf(stderr, "More than one operation "
 					"specified. Aborting.\n");
-				exit(1);
+				cli_classic_abort_usage(argv[0]);
 			}
+			filename = strdup(optarg);
 			read_it = 1;
 			break;
 		case 'w':
 			if (++operation_specified > 1) {
 				fprintf(stderr, "More than one operation "
 					"specified. Aborting.\n");
-				exit(1);
+				cli_classic_abort_usage(argv[0]);
 			}
+			filename = strdup(optarg);
 			write_it = 1;
 			break;
 		case 'v':
@@ -183,20 +203,21 @@ int cli_classic(int argc, char *argv[])
 			if (++operation_specified > 1) {
 				fprintf(stderr, "More than one operation "
 					"specified. Aborting.\n");
-				exit(1);
+				cli_classic_abort_usage(argv[0]);
 			}
 			if (dont_verify_it) {
 				fprintf(stderr, "--verify and --noverify are"
 					"mutually exclusive. Aborting.\n");
-				exit(1);
+				cli_classic_abort_usage(argv[0]);
 			}
+			filename = strdup(optarg);
 			verify_it = 1;
 			break;
 		case 'n':
 			if (verify_it) {
 				fprintf(stderr, "--verify and --noverify are"
 					"mutually exclusive. Aborting.\n");
-				exit(1);
+				cli_classic_abort_usage(argv[0]);
 			}
 			dont_verify_it = 1;
 			break;
@@ -210,36 +231,55 @@ int cli_classic(int argc, char *argv[])
 			if (++operation_specified > 1) {
 				fprintf(stderr, "More than one operation "
 					"specified. Aborting.\n");
-				exit(1);
+				cli_classic_abort_usage(argv[0]);
 			}
 			erase_it = 1;
 			break;
-#if INTERNAL_SUPPORT == 1
 		case 'm':
+#if INTERNAL_SUPPORT == 1
 			tempstr = strdup(optarg);
 			lb_vendor_dev_from_string(tempstr);
-			break;
+#else
+			fprintf(stderr, "Error: Internal programmer support "
+				"was not compiled in and --mainboard only\n"
+				"applies to the internal programmer. Aborting.\n");
+			cli_classic_abort_usage(argv[0]);
 #endif
+			break;
 		case 'f':
 			force = 1;
 			break;
 		case 'l':
 			tempstr = strdup(optarg);
 			if (read_romlayout(tempstr))
-				exit(1);
+				cli_classic_abort_usage(argv[0]);
 			break;
 		case 'i':
 			tempstr = strdup(optarg);
 			find_romentry(tempstr);
 			break;
 		case 'L':
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation "
+					"specified. Aborting.\n");
+				cli_classic_abort_usage(argv[0]);
+			}
 			list_supported = 1;
 			break;
-#if PRINT_WIKI_SUPPORT == 1
 		case 'z':
+#if PRINT_WIKI_SUPPORT == 1
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation "
+					"specified. Aborting.\n");
+				cli_classic_abort_usage(argv[0]);
+			}
 			list_supported_wiki = 1;
-			break;
+#else
+			fprintf(stderr, "Error: Wiki output was not compiled "
+				"in. Aborting.\n");
+			cli_classic_abort_usage(argv[0]);
 #endif
+			break;
 		case 'p':
 			for (programmer = 0; programmer < PROGRAMMER_INVALID; programmer++) {
 				name = programmer_table[programmer].name;
@@ -267,21 +307,37 @@ int cli_classic(int argc, char *argv[])
 				}
 			}
 			if (programmer == PROGRAMMER_INVALID) {
-				printf("Error: Unknown programmer %s.\n", optarg);
-				exit(1);
+				fprintf(stderr, "Error: Unknown programmer "
+					"%s.\n", optarg);
+				cli_classic_abort_usage(argv[0]);
 			}
 			break;
 		case 'R':
 			/* print_version() is always called during startup. */
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation "
+					"specified. Aborting.\n");
+				cli_classic_abort_usage(argv[0]);
+			}
 			exit(0);
 			break;
 		case 'h':
-		default:
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation "
+					"specified. Aborting.\n");
+				cli_classic_abort_usage(argv[0]);
+			}
 			cli_classic_usage(argv[0]);
+			exit(0);
+			break;
+		default:
+			cli_classic_abort_usage(argv[0]);
 			break;
 		}
 	}
 
+	/* FIXME: Print the actions flashrom will take. */
+
 	if (list_supported) {
 		print_supported();
 		exit(0);
@@ -294,18 +350,18 @@ int cli_classic(int argc, char *argv[])
 	}
 #endif
 
-	if (read_it && write_it) {
-		printf("Error: -r and -w are mutually exclusive.\n");
-		cli_classic_usage(argv[0]);
+	if (optind < argc) {
+		fprintf(stderr, "Error: Extra parameter found.\n");
+		cli_classic_abort_usage(argv[0]);
 	}
 
-	if (optind < argc)
-		filename = argv[optind++];
-	
-	if (optind < argc) {
-		printf("Error: Extra parameter found.\n");
-		cli_classic_usage(argv[0]);
+#if INTERNAL_SUPPORT == 1
+	if ((programmer != PROGRAMMER_INTERNAL) && (lb_part || lb_vendor)) {
+		fprintf(stderr, "Error: --mainboard requires the internal "
+				"programmer. Aborting.\n");
+		cli_classic_abort_usage(argv[0]);
 	}
+#endif
 
 	if (chip_to_probe) {
 		for (flash = flashchips; flash && flash->name; flash++)
@@ -321,13 +377,15 @@ int cli_classic(int argc, char *argv[])
 		/* Clean up after the check. */
 		flash = NULL;
 	}
-		
+
+	msg_pdbg("Initializing %s programmer\n",
+		 programmer_table[programmer].name);
 	if (programmer_init()) {
 		fprintf(stderr, "Error: Programmer initialization failed.\n");
 		exit(1);
 	}
 
-	// FIXME: Delay calibration should happen in programmer code.
+	/* FIXME: Delay calibration should happen in programmer code. */
 	myusec_calibrate_delay();
 
 	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
@@ -383,7 +441,7 @@ int cli_classic(int argc, char *argv[])
 		printf("No operations were specified.\n");
 		// FIXME: flash writes stay enabled!
 		programmer_shutdown();
-		exit(1);
+		exit(0);
 	}
 
 	if (!filename && !erase_it) {