On Fri, Jul 15, 2011 at 4:06 PM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@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@gmx.net>

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@flashrom.org
http://www.flashrom.org/mailman/listinfo/flashrom