Reviewers: ulan,

Message:
PTAL.

Description:
Ensure removing processed command line arguments.


BUG="d8 --crankshaft --expose-debug-as" crashes
TEST=test-flags/FlagsRemoveIncomplete


Please review this at http://codereview.chromium.org/10534137/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/flags.cc
  M test/cctest/test-flags.cc


Index: src/flags.cc
diff --git a/src/flags.cc b/src/flags.cc
index 5720cbda346b00993b8caa00335cc125ae672321..14c230a2d093581ba79dc7d3f83145016b1d20cd 100644
--- a/src/flags.cc
+++ b/src/flags.cc
@@ -343,6 +343,7 @@ static Flag* FindFlag(const char* name) {
 int FlagList::SetFlagsFromCommandLine(int* argc,
                                       char** argv,
                                       bool remove_flags) {
+  int return_code = 0;
   // parse arguments
   for (int i = 1; i < *argc;) {
     int j = i;  // j > 0
@@ -368,7 +369,8 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
         } else {
           fprintf(stderr, "Error: unrecognized flag %s\n"
                   "Try --help for options\n", arg);
-          return j;
+          return_code = j;
+          break;
         }
       }

@@ -382,7 +384,8 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
           fprintf(stderr, "Error: missing value for flag %s of type %s\n"
                   "Try --help for options\n",
                   arg, Type2String(flag->type()));
-          return j;
+          return_code = j;
+          break;
         }
       }

@@ -424,7 +427,8 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
         fprintf(stderr, "Error: illegal value for flag %s of type %s\n"
                 "Try --help for options\n",
                 arg, Type2String(flag->type()));
-        return j;
+        return_code = j;
+        break;
       }

       // remove the flag & value from the command
@@ -451,7 +455,7 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
     exit(0);
   }
   // parsed all flags successfully
-  return 0;
+  return return_code;
 }


Index: test/cctest/test-flags.cc
diff --git a/test/cctest/test-flags.cc b/test/cctest/test-flags.cc
index 32f1264f7f1ab7a8fd783596ca60dbeff1f0d1e8..f3a33a7067e9aa2d60f18ba66aeb8cb7025e7799 100644
--- a/test/cctest/test-flags.cc
+++ b/test/cctest/test-flags.cc
@@ -232,3 +232,20 @@ TEST(FlagsJSArguments4) {
   CHECK_EQ(0, FLAG_js_arguments.argc());
 }

+
+TEST(FlagsRemoveIncomplete) {
+  // Test that processed command line arguments are removed, even
+  // if the list of arguments ends unexpectedly.
+  SetFlagsToDefault();
+  int argc = 3;
+  char* argv[3];
+  char* arg0 = const_cast<char*>("");
+  char* arg1 = const_cast<char*>("--crankshaft");
+  char* arg2 = const_cast<char*>("--expose-debug-as");
+  argv[0] = arg0;
+  argv[1] = arg1;
+  argv[2] = arg2;
+  CHECK_EQ(2, FlagList::SetFlagsFromCommandLine(&argc, argv, true));
+  CHECK_NE(NULL, argv[1]);
+  CHECK_EQ(argc, 2);
+}


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to