flashrom does not honor argument ordering for operations. Not only does
this violate the principle of least surprise, it also caused one bug
where -Ewv was specified and the flash ended up being empty.

Support only one operation at a time.
As a side benefit, this allows us to clean up main() quite a bit.

Signed-off-by: Carl-Daniel Hailfinger <[email protected]>

Index: flashrom-one_commandline_operation_only/flashrom.8
===================================================================
--- flashrom-one_commandline_operation_only/flashrom.8  (Revision 583)
+++ flashrom-one_commandline_operation_only/flashrom.8  (Arbeitskopie)
@@ -2,7 +2,7 @@
 .SH NAME
 flashrom \- detect, read, write, verify and erase flash chips
 .SH SYNOPSIS
-.B flashrom \fR[\fB\-EVfLhR\fR] [\fB\-r\fR file] [\fB\-w\fR file] [\fB\-v\fR 
file]
+.B flashrom \fR[\fB\-VfLhR\fR] [\fB\-E\fR|\fB\-r\fR file|\fB\-w\fR 
file|\fB\-v\fR file]
          [\fB\-c\fR chipname] [\fB\-s\fR addr] [\fB\-e\fR addr] [\fB\-m\fR 
[vendor:]part]
          [\fB\-l\fR file] [\fB\-i\fR image] [\fB\-p\fR programmer] [file]
 .SH DESCRIPTION
@@ -25,7 +25,8 @@
 flashrom 1.0. Do not use flashrom in scripts or other automated tools without
 checking that your flashrom version won't interpret options in a different way.
 .PP
-If no file is specified, then all that happens
+You can specify one of -E, -r, -w, -v or no operation.
+If no operation is specified, then all that happens
 is that flash info is dumped and the flash chip is set to writable.
 .TP
 .B "\-r, \-\-read <file>"
Index: flashrom-one_commandline_operation_only/flashrom.c
===================================================================
--- flashrom-one_commandline_operation_only/flashrom.c  (Revision 583)
+++ flashrom-one_commandline_operation_only/flashrom.c  (Arbeitskopie)
@@ -471,7 +471,7 @@
 
 void usage(const char *name)
 {
-       printf("usage: %s [-EVfLhR] [-r file] [-w file] [-v file] [-c chipname] 
[-s addr]\n"
+       printf("usage: %s [-VfLhR] [-E|-r file|-w file|-v file] [-c chipname] 
[-s addr]\n"
               "       [-e addr] [-m [vendor:]part] [-l file] [-i image] [-p 
programmer] [file]\n\n",
               name);
 
@@ -498,7 +498,8 @@
             "                                     (internal, dummy, nic3com, 
satasii, it87spi)\n"
             "   -h | --help:                      print this help text\n"
             "   -R | --version:                   print the version 
(release)\n"
-            "\nIf no file is specified, then all that happens"
+            "\nYou can specify one of -E, -r, -w, -v or no operation.\n"
+            "If no operation is specified, then all that happens"
             " is that flash info is dumped.\n\n");
        exit(1);
 }
@@ -520,6 +521,7 @@
        int force = 0;
        int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
        int list_supported = 0;
+       int operation_specified = 0;
        int ret = 0, i;
 
        static struct option long_options[] = {
@@ -562,12 +564,27 @@
                                  long_options, &option_index)) != EOF) {
                switch (opt) {
                case 'r':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        read_it = 1;
                        break;
                case 'w':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        write_it = 1;
                        break;
                case 'v':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        verify_it = 1;
                        break;
                case 'c':
@@ -577,6 +594,11 @@
                        verbose = 1;
                        break;
                case 'E':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        erase_it = 1;
                        break;
                case 's':


-- 
http://www.hailfinger.org/

Index: flashrom-one_commandline_operation_only/flashrom.8
===================================================================
--- flashrom-one_commandline_operation_only/flashrom.8  (Revision 583)
+++ flashrom-one_commandline_operation_only/flashrom.8  (Arbeitskopie)
@@ -2,7 +2,7 @@
 .SH NAME
 flashrom \- detect, read, write, verify and erase flash chips
 .SH SYNOPSIS
-.B flashrom \fR[\fB\-EVfLhR\fR] [\fB\-r\fR file] [\fB\-w\fR file] [\fB\-v\fR 
file]
+.B flashrom \fR[\fB\-VfLhR\fR] [\fB\-E\fR|\fB\-r\fR file|\fB\-w\fR 
file|\fB\-v\fR file]
          [\fB\-c\fR chipname] [\fB\-s\fR addr] [\fB\-e\fR addr] [\fB\-m\fR 
[vendor:]part]
          [\fB\-l\fR file] [\fB\-i\fR image] [\fB\-p\fR programmer] [file]
 .SH DESCRIPTION
@@ -25,7 +25,8 @@
 flashrom 1.0. Do not use flashrom in scripts or other automated tools without
 checking that your flashrom version won't interpret options in a different way.
 .PP
-If no file is specified, then all that happens
+You can specify one of -E, -r, -w, -v or no operation.
+If no operation is specified, then all that happens
 is that flash info is dumped and the flash chip is set to writable.
 .TP
 .B "\-r, \-\-read <file>"
Index: flashrom-one_commandline_operation_only/flashrom.c
===================================================================
--- flashrom-one_commandline_operation_only/flashrom.c  (Revision 583)
+++ flashrom-one_commandline_operation_only/flashrom.c  (Arbeitskopie)
@@ -471,7 +471,7 @@
 
 void usage(const char *name)
 {
-       printf("usage: %s [-EVfLhR] [-r file] [-w file] [-v file] [-c chipname] 
[-s addr]\n"
+       printf("usage: %s [-VfLhR] [-E|-r file|-w file|-v file] [-c chipname] 
[-s addr]\n"
               "       [-e addr] [-m [vendor:]part] [-l file] [-i image] [-p 
programmer] [file]\n\n",
               name);
 
@@ -498,7 +498,8 @@
             "                                     (internal, dummy, nic3com, 
satasii, it87spi)\n"
             "   -h | --help:                      print this help text\n"
             "   -R | --version:                   print the version 
(release)\n"
-            "\nIf no file is specified, then all that happens"
+            "\nYou can specify one of -E, -r, -w, -v or no operation.\n"
+            "If no operation is specified, then all that happens"
             " is that flash info is dumped.\n\n");
        exit(1);
 }
@@ -520,6 +521,7 @@
        int force = 0;
        int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
        int list_supported = 0;
+       int operation_specified = 0;
        int ret = 0, i;
 
        static struct option long_options[] = {
@@ -562,12 +564,27 @@
                                  long_options, &option_index)) != EOF) {
                switch (opt) {
                case 'r':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        read_it = 1;
                        break;
                case 'w':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        write_it = 1;
                        break;
                case 'v':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        verify_it = 1;
                        break;
                case 'c':
@@ -577,6 +594,11 @@
                        verbose = 1;
                        break;
                case 'E':
+                       if (++operation_specified > 1) {
+                               fprintf(stderr, "More than one operation "
+                                       "specified. Aborting.\n");
+                               exit(1);
+                       }
                        erase_it = 1;
                        break;
                case 's':
-- 
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to