[flashrom] [PATCH] Fix and clean up cli_classic.c
David Hendricks
dhendrix at google.com
Sat Jul 16 01:43:17 CEST 2011
On Fri, Jul 15, 2011 at 4:06 PM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2006 at gmx.net> 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 <c-d.hailfinger.devel.2006 at gmx.net>
>
Looks good to me.
Acked-by: David Hendricks <dhendrix at google.com>
>
> 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
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
>
--
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20110715/9b23105e/attachment.html>
More information about the flashrom
mailing list