On Fri, Jul 15, 2011 at 4:06 PM, Carl-Daniel Hailfinger < [email protected]> wrote:
> Don't ignore -i if it is specified before -l > Check if image mentioned by -i is present in layout file > Clean up cli_classic.c: > - Consolidate duplicated programmer_shutdown calls > - Kill outdated comments > - Finish parameter checking before -L/-z is executed > This patch was inspired by a cleanup patch by Stefan Tauner. > I did not fix up the -i/-l ordering dependency because the new layout > code may have different requirements anyway, and introducing gratuitous > user interface changes would be a bad idea. > Side note: This also reduces the amount of code changes needed for the > logfile patch. > > Signed-off-by: Carl-Daniel Hailfinger <[email protected]> > Looks good to me. Acked-by: David Hendricks <[email protected]> > > Index: flashrom-cli_classic_cleanup/cli_classic.c > =================================================================== > --- flashrom-cli_classic_cleanup/cli_classic.c (Revision 1372) > +++ flashrom-cli_classic_cleanup/cli_classic.c (Arbeitskopie) > @@ -117,6 +117,7 @@ > #endif > int operation_specified = 0; > int i; > + int ret = 0; > > static const char optstring[] = "r:Rw:v:nVEfc:m:l:i:p:Lzh"; > static const struct option long_options[] = { > @@ -232,8 +233,14 @@ > cli_classic_abort_usage(); > break; > case 'i': > + /* FIXME: -l has to be specified before -i. */ > tempstr = strdup(optarg); > - find_romentry(tempstr); > + if (find_romentry(tempstr)) { > + fprintf(stderr, "Error: image %s not found > in " > + "layout file or -i specified before > " > + "-l\n", tempstr); > + cli_classic_abort_usage(); > + } > break; > case 'L': > if (++operation_specified > 1) { > @@ -313,6 +320,11 @@ > } > } > > + if (optind < argc) { > + fprintf(stderr, "Error: Extra parameter found.\n"); > + cli_classic_abort_usage(); > + } > + > /* FIXME: Print the actions flashrom will take. */ > > if (list_supported) { > @@ -327,11 +339,6 @@ > } > #endif > > - if (optind < argc) { > - fprintf(stderr, "Error: Extra parameter found.\n"); > - cli_classic_abort_usage(); > - } > - > #if CONFIG_INTERNAL == 1 > if ((programmer != PROGRAMMER_INTERNAL) && (lb_part || lb_vendor)) { > fprintf(stderr, "Error: --mainboard requires the internal " > @@ -360,8 +367,8 @@ > > if (programmer_init(pparam)) { > fprintf(stderr, "Error: Programmer initialization > failed.\n"); > - programmer_shutdown(); > - exit(1); > + ret = 1; > + goto out_shutdown; > } > > for (i = 0; i < ARRAY_SIZE(flashes); i++) { > @@ -377,8 +384,8 @@ > for (i = 0; i < chipcount; i++) > printf(" %s", flashes[i].name); > printf("\nPlease specify which chip to use with the -c > <chipname> option.\n"); > - programmer_shutdown(); > - exit(1); > + ret = 1; > + goto out_shutdown; > } else if (!chipcount) { > printf("No EEPROM/flash device found.\n"); > if (!force || !chip_to_probe) { > @@ -389,15 +396,14 @@ > startchip = probe_flash(0, &flashes[0], 1); > if (startchip == -1) { > printf("Probing for flash chip '%s' > failed.\n", chip_to_probe); > - programmer_shutdown(); > - exit(1); > + ret = 1; > + goto out_shutdown; > } > printf("Please note that forced reads most likely > contain garbage.\n"); > return read_flash_to_file(&flashes[0], filename); > } > - // FIXME: flash writes stay enabled! > - programmer_shutdown(); > - exit(1); > + ret = 1; > + goto out_shutdown; > } > > fill_flash = &flashes[0]; > @@ -409,22 +415,19 @@ > (!force)) { > fprintf(stderr, "Chip is too big for this programmer " > "(-V gives details). Use --force to override.\n"); > - programmer_shutdown(); > - return 1; > + ret = 1; > + goto out_shutdown; > } > > if (!(read_it | write_it | verify_it | erase_it)) { > printf("No operations were specified.\n"); > - // FIXME: flash writes stay enabled! > - programmer_shutdown(); > - exit(0); > + goto out_shutdown; > } > > if (!filename && !erase_it) { > printf("Error: No filename specified.\n"); > - // FIXME: flash writes stay enabled! > - programmer_shutdown(); > - exit(1); > + ret = 1; > + goto out_shutdown; > } > > /* Always verify write operations unless -n is used. */ > @@ -437,4 +440,8 @@ > */ > programmer_delay(100000); > return doit(fill_flash, force, filename, read_it, write_it, > erase_it, verify_it); > + > +out_shutdown: > + programmer_shutdown(); > + return ret; > } > > > -- > http://www.hailfinger.org/ > > > _______________________________________________ > flashrom mailing list > [email protected] > http://www.flashrom.org/mailman/listinfo/flashrom > -- David Hendricks (dhendrix) Systems Software Engineer, Google Inc.
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
