From 8841d3e703e3d3f7c7920b7f9439fc9333c15638 Mon Sep 17 00:00:00 2001 From: Carl-Daniel Hailfinger Date: Sat, 15 May 2010 15:04:37 +0000 Subject: 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 Acked-by: Michael Karcher --- cli_classic.c | 202 +++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 130 insertions(+), 72 deletions(-) (limited to 'cli_classic.c') 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 : 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 |-w |-v ]\n" + " [-c ] [-m [:]] [-l ]\n" + " [-i ] [-p [:]]\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 read flash and save to " + "\n" + " -w | --write write to flash\n" + " -v | --verify verify flash against " + "\n" + " -E | --erase erase flash device\n" + " -V | --verbose more verbose output\n" + " -c | --chip 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 : read ROM layout from file\n" - " -i | --image : 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 read ROM layout from " + "\n" + " -i | --image only flash image " + "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 : specify the programmer device"); + " -p | --programmer [:] 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) { -- cgit v1.1