I have self-acked and committed a smaller portion of this in r1788. The remaining parts are attached. I think we will need a complete new concept of handling control input with libflashrom, so I probably wont try getting this in.
-- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
>From 3997dffde2789b7c904f8a9f93a74fd5513220c4 Mon Sep 17 00:00:00 2001 From: Stefan Tauner <[email protected]> Date: Wed, 7 May 2014 18:26:13 +0200 Subject: [PATCH] Reorganize shutting down. This was mainly driven by getting rid of cli_classic_abort_usage() in cli_classic.c. This makes the whole main function more consistent and fixes a myriad of (theoretical) memory leaks. This slightly changes output in most error cases. Signed-off-by: Stefan Tauner <[email protected]> --- cli_classic.c | 139 ++++++++++++++++++++++++---------------------------------- 1 file changed, 57 insertions(+), 82 deletions(-) diff --git a/cli_classic.c b/cli_classic.c index a32d55b..2d309c0 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -71,12 +71,6 @@ static void cli_classic_usage(const char *name) "If no operation is specified, flashrom will only probe for flash chips.\n"); } -static void cli_classic_abort_usage(void) -{ - printf("Please run \"flashrom --help\" for usage info.\n"); - exit(1); -} - static int check_filename(char *filename, char *type) { if (!filename || (filename[0] == '\0')) { @@ -151,40 +145,36 @@ int main(int argc, char *argv[]) switch (opt) { case 'r': if (++operation_specified > 1) { - fprintf(stderr, "More than one operation " - "specified. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } filename = strdup(optarg); read_it = 1; break; case 'w': if (++operation_specified > 1) { - fprintf(stderr, "More than one operation " - "specified. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } filename = strdup(optarg); write_it = 1; break; case 'v': - //FIXME: gracefully handle superfluous -v if (++operation_specified > 1) { - fprintf(stderr, "More than one operation " - "specified. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } if (dont_verify_it) { - fprintf(stderr, "--verify and --noverify are mutually exclusive. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "--verify and --noverify are mutually exclusive.\n"); + goto out_abort; } filename = strdup(optarg); verify_it = 1; break; case 'n': if (verify_it) { - fprintf(stderr, "--verify and --noverify are mutually exclusive. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "--verify and --noverify are mutually exclusive.\n"); + goto out_abort; } dont_verify_it = 1; break; @@ -198,9 +188,8 @@ int main(int argc, char *argv[]) break; case 'E': if (++operation_specified > 1) { - fprintf(stderr, "More than one operation " - "specified. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } erase_it = 1; break; @@ -209,9 +198,8 @@ int main(int argc, char *argv[]) break; case 'l': if (layoutfile) { - fprintf(stderr, "Error: --layout specified " - "more than once. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "Error: --layout specified more than once.\n"); + goto out_abort; } layoutfile = strdup(optarg); break; @@ -219,29 +207,26 @@ int main(int argc, char *argv[]) tempstr = strdup(optarg); if (register_include_arg(tempstr)) { free(tempstr); - cli_classic_abort_usage(); + goto out_abort; } break; case 'L': if (++operation_specified > 1) { - fprintf(stderr, "More than one operation " - "specified. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } list_supported = 1; break; case 'z': #if CONFIG_PRINT_WIKI == 1 if (++operation_specified > 1) { - fprintf(stderr, "More than one operation " - "specified. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } list_supported_wiki = 1; #else - fprintf(stderr, "Error: Wiki output was not compiled " - "in. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "Error: Wiki output was not compiled in.\n"); + goto out_abort; #endif break; case 'p': @@ -251,7 +236,7 @@ int main(int argc, char *argv[]) "multiple\nparameters for a programmer " "with \",\". Please see the man page " "for details.\n"); - cli_classic_abort_usage(); + goto out_abort; } for (prog = 0; prog < PROGRAMMER_INVALID; prog++) { name = programmer_table[prog].name; @@ -283,62 +268,57 @@ int main(int argc, char *argv[]) optarg); list_programmers_linebreak(0, 80, 0); msg_ginfo(".\n"); - cli_classic_abort_usage(); + goto out_abort; } 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(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } - exit(0); - break; + goto out; case 'h': if (++operation_specified > 1) { - fprintf(stderr, "More than one operation " - "specified. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "More than one operation specified.\n"); + goto out_abort; } cli_classic_usage(argv[0]); - exit(0); - break; + goto out; case 'o': #ifdef STANDALONE - fprintf(stderr, "Log file not supported in standalone mode. Aborting.\n"); - cli_classic_abort_usage(); + fprintf(stderr, "Log file not supported in standalone mode.\n"); + goto out_abort; #else /* STANDALONE */ logfile = strdup(optarg); if (logfile[0] == '\0') { fprintf(stderr, "No log filename specified.\n"); - cli_classic_abort_usage(); + goto out_abort; } #endif /* STANDALONE */ break; default: - cli_classic_abort_usage(); - break; + goto out_abort; } } if (optind < argc) { fprintf(stderr, "Error: Extra parameter found.\n"); - cli_classic_abort_usage(); + goto out_abort; } if ((read_it | write_it | verify_it) && check_filename(filename, "image")) { - cli_classic_abort_usage(); + goto out_abort; } if (layoutfile && check_filename(layoutfile, "layout")) { - cli_classic_abort_usage(); + goto out_abort; } #ifndef STANDALONE if (logfile && check_filename(logfile, "log")) - cli_classic_abort_usage(); + goto out_abort; if (logfile && open_logfile(logfile)) - cli_classic_abort_usage(); + goto out_abort; #endif /* !STANDALONE */ #if CONFIG_PRINT_WIKI == 1 @@ -350,7 +330,7 @@ int main(int argc, char *argv[]) if (list_supported) { if (print_supported()) - ret = 1; + goto out_abort; goto out; } @@ -366,8 +346,7 @@ int main(int argc, char *argv[]) msg_gdbg("\n"); if (layoutfile && read_romlayout(layoutfile)) { - ret = 1; - goto out; + goto out_abort; } if (layoutfile != NULL && !write_it) { msg_gerr("Layout files are currently supported for write operations only.\n"); @@ -376,8 +355,7 @@ int main(int argc, char *argv[]) } if (process_include_args()) { - ret = 1; - goto out; + goto out_abort; } /* Does a chip with the requested name exist in the flashchips array? */ if (chip_to_probe) { @@ -387,8 +365,7 @@ int main(int argc, char *argv[]) if (!chip || !chip->name) { msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe); msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n"); - ret = 1; - goto out; + goto out_abort; } /* Keep chip around for later usage in case a forced read is requested. */ } @@ -407,8 +384,7 @@ int main(int argc, char *argv[]) "Valid choices are:\n"); list_programmers_linebreak(0, 80, 0); msg_ginfo(".\n"); - ret = 1; - goto out; + goto out_abort; } } @@ -418,7 +394,7 @@ int main(int argc, char *argv[]) if (programmer_init(prog, pparam)) { msg_perr("Error: Programmer initialization failed.\n"); ret = 1; - goto out_shutdown; + goto out; /* Lots of init code prints "Aborting." at the end. Avoid printing it again here. */ } tempstr = flashbuses_to_text(get_buses_supported()); msg_pdbg("The following protocols are supported: %s.\n", tempstr); @@ -441,8 +417,7 @@ int main(int argc, char *argv[]) for (i = 1; i < chipcount; i++) msg_cinfo(", \"%s\"", flashes[i].chip->name); msg_cinfo("\nPlease specify which chip definition to use with the -c <chipname> option.\n"); - ret = 1; - goto out_shutdown; + goto out_abort; } else if (!chipcount) { msg_cinfo("No EEPROM/flash device found.\n"); if (!force || !chip_to_probe) { @@ -462,8 +437,7 @@ int main(int argc, char *argv[]) } if (!compatible_programmers) { msg_cinfo("No compatible controller found for the requested flash chip.\n"); - ret = 1; - goto out_shutdown; + goto out_abort; } if (compatible_programmers > 1) msg_cinfo("More than one compatible controller found for the requested flash " @@ -477,16 +451,14 @@ int main(int argc, char *argv[]) if (startchip == -1) { // FIXME: This should never happen! Ask for a bug report? msg_cinfo("Probing for flash chip '%s' failed.\n", chip_to_probe); - ret = 1; - goto out_shutdown; + goto out_abort; } msg_cinfo("Please note that forced reads most likely contain garbage.\n"); ret = read_flash_to_file(&flashes[0], filename); free(flashes[0].chip); - goto out_shutdown; + goto out; } - ret = 1; - goto out_shutdown; + goto out_abort; } else if (!chip_to_probe) { /* repeat for convenience when looking at foreign logs */ tempstr = flashbuses_to_text(flashes[0].chip->bustype); @@ -502,13 +474,12 @@ int main(int argc, char *argv[]) size = fill_flash->chip->total_size * 1024; if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->chip->bustype, size) && (!force)) { msg_cerr("Chip is too big for this programmer (-V gives details). Use --force to override.\n"); - ret = 1; - goto out_shutdown; + goto out_abort; } if (!(read_it | write_it | verify_it | erase_it)) { msg_ginfo("No operations were specified.\n"); - goto out_shutdown; + goto out; } /* Always verify write operations unless -n is used. */ @@ -521,10 +492,14 @@ int main(int argc, char *argv[]) */ programmer_delay(100000); ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it); + goto out; -out_shutdown: - programmer_shutdown(); +out_abort: + msg_gerr("Aborting.\n"); + ret = 1; out: + programmer_shutdown(); + for (i = 0; i < chipcount; i++) free(flashes[i].chip); -- Kind regards, Stefan Tauner
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
