[driver] Fix --core/-c and add test
authorJonas Devlieghere <jonas@devlieghere.com>
Thu, 29 Nov 2018 00:22:28 +0000 (00:22 +0000)
committerJonas Devlieghere <jonas@devlieghere.com>
Thu, 29 Nov 2018 00:22:28 +0000 (00:22 +0000)
Because the optarg variable was shadowed we didn't notice we weren't
extracting the value from the option. This patch fixes that and renames
the variable to prevent this from happening in the future.

I also added two tests to check the error output for --core and --file
when the given value doesn't exist.

llvm-svn: 347821

lldb/lit/Driver/TestCore.test [new file with mode: 0644]
lldb/lit/Driver/TestFile.test [new file with mode: 0644]
lldb/tools/driver/Driver.cpp
lldb/tools/driver/Options.td

diff --git a/lldb/lit/Driver/TestCore.test b/lldb/lit/Driver/TestCore.test
new file mode 100644 (file)
index 0000000..cca8171
--- /dev/null
@@ -0,0 +1,2 @@
+# RUN: not %lldb -c /bogus/path 2>&1 | FileCheck %s
+# CHECK: error: file specified in --core (-c) option doesn't exist
diff --git a/lldb/lit/Driver/TestFile.test b/lldb/lit/Driver/TestFile.test
new file mode 100644 (file)
index 0000000..0e80594
--- /dev/null
@@ -0,0 +1,2 @@
+# RUN: not %lldb -f /bogus/path 2>&1 | FileCheck %s
+# CHECK: error: file specified in --file (-f) option doesn't exist
index e84fff1..6371030 100644 (file)
@@ -307,15 +307,16 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
     m_option_data.m_batch = true;
   }
 
-  if (args.hasArg(OPT_core)) {
-    SBFileSpec file(optarg);
-    if (file.Exists()) {
-      m_option_data.m_core_file = optarg;
-    } else {
+  if (auto *arg = args.getLastArg(OPT_core)) {
+    auto arg_value = arg->getValue();
+    SBFileSpec file(arg_value);
+    if (!file.Exists()) {
       error.SetErrorStringWithFormat(
-          "file specified in --core (-c) option doesn't exist: '%s'", optarg);
+          "file specified in --core (-c) option doesn't exist: '%s'",
+          arg_value);
       return error;
     }
+    m_option_data.m_core_file = arg_value;
   }
 
   if (args.hasArg(OPT_editor)) {
@@ -332,33 +333,34 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   }
 
   if (auto *arg = args.getLastArg(OPT_file)) {
-    auto optarg = arg->getValue();
-    SBFileSpec file(optarg);
+    auto arg_value = arg->getValue();
+    SBFileSpec file(arg_value);
     if (file.Exists()) {
-      m_option_data.m_args.push_back(optarg);
+      m_option_data.m_args.push_back(arg_value);
     } else if (file.ResolveExecutableLocation()) {
       char path[PATH_MAX];
       file.GetPath(path, sizeof(path));
       m_option_data.m_args.push_back(path);
     } else {
       error.SetErrorStringWithFormat(
-          "file specified in --file (-f) option doesn't exist: '%s'", optarg);
+          "file specified in --file (-f) option doesn't exist: '%s'",
+          arg_value);
       return error;
     }
   }
 
   if (auto *arg = args.getLastArg(OPT_arch)) {
-    auto optarg = arg->getValue();
-    if (!m_debugger.SetDefaultArchitecture(optarg)) {
+    auto arg_value = arg->getValue();
+    if (!m_debugger.SetDefaultArchitecture(arg_value)) {
       error.SetErrorStringWithFormat(
-          "invalid architecture in the -a or --arch option: '%s'", optarg);
+          "invalid architecture in the -a or --arch option: '%s'", arg_value);
       return error;
     }
   }
 
   if (auto *arg = args.getLastArg(OPT_script_language)) {
-    auto optarg = arg->getValue();
-    m_option_data.m_script_lang = m_debugger.GetScriptingLanguage(optarg);
+    auto arg_value = arg->getValue();
+    m_option_data.m_script_lang = m_debugger.GetScriptingLanguage(arg_value);
   }
 
   if (args.hasArg(OPT_no_use_colors)) {
@@ -366,16 +368,16 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   }
 
   if (auto *arg = args.getLastArg(OPT_reproducer)) {
-    auto optarg = arg->getValue();
-    SBFileSpec file(optarg);
+    auto arg_value = arg->getValue();
+    SBFileSpec file(arg_value);
     if (file.Exists()) {
-      SBError repro_error = m_debugger.ReplayReproducer(optarg);
+      SBError repro_error = m_debugger.ReplayReproducer(arg_value);
       if (repro_error.Fail())
         return repro_error;
     } else {
       error.SetErrorStringWithFormat("file specified in --reproducer "
                                      "(-z) option doesn't exist: '%s'",
-                                     optarg);
+                                     arg_value);
       return error;
     }
   }
@@ -389,8 +391,8 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   }
 
   if (auto *arg = args.getLastArg(OPT_attach_name)) {
-    auto optarg = arg->getValue();
-    m_option_data.m_process_name = optarg;
+    auto arg_value = arg->getValue();
+    m_option_data.m_process_name = arg_value;
   }
 
   if (args.hasArg(OPT_wait_for)) {
@@ -398,32 +400,32 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   }
 
   if (auto *arg = args.getLastArg(OPT_attach_pid)) {
-    auto optarg = arg->getValue();
+    auto arg_value = arg->getValue();
     char *remainder;
-    m_option_data.m_process_pid = strtol(optarg, &remainder, 0);
-    if (remainder == optarg || *remainder != '\0') {
+    m_option_data.m_process_pid = strtol(arg_value, &remainder, 0);
+    if (remainder == arg_value || *remainder != '\0') {
       error.SetErrorStringWithFormat(
-          "Could not convert process PID: \"%s\" into a pid.", optarg);
+          "Could not convert process PID: \"%s\" into a pid.", arg_value);
       return error;
     }
   }
 
   if (auto *arg = args.getLastArg(OPT_repl_language)) {
-    auto optarg = arg->getValue();
+    auto arg_value = arg->getValue();
     m_option_data.m_repl_lang =
-        SBLanguageRuntime::GetLanguageTypeFromString(optarg);
+        SBLanguageRuntime::GetLanguageTypeFromString(arg_value);
     if (m_option_data.m_repl_lang == eLanguageTypeUnknown) {
       error.SetErrorStringWithFormat("Unrecognized language name: \"%s\"",
-                                     optarg);
+                                     arg_value);
       return error;
     }
   }
 
   if (auto *arg = args.getLastArg(OPT_repl)) {
-    auto optarg = arg->getValue();
+    auto arg_value = arg->getValue();
     m_option_data.m_repl = true;
-    if (optarg && optarg[0])
-      m_option_data.m_repl_options = optarg;
+    if (arg_value && arg_value[0])
+      m_option_data.m_repl_options = arg_value;
     else
       m_option_data.m_repl_options.clear();
   }
@@ -433,49 +435,44 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   for (auto *arg : args.filtered(OPT_source_on_crash, OPT_one_line_on_crash,
                                  OPT_source, OPT_source_before_file,
                                  OPT_one_line, OPT_one_line_before_file)) {
+    auto arg_value = arg->getValue();
     if (arg->getOption().matches(OPT_source_on_crash)) {
-      auto optarg = arg->getValue();
-      m_option_data.AddInitialCommand(optarg, eCommandPlacementAfterCrash, true,
-                                      error);
+      m_option_data.AddInitialCommand(arg_value, eCommandPlacementAfterCrash,
+                                      true, error);
       if (error.Fail())
         return error;
     }
 
     if (arg->getOption().matches(OPT_one_line_on_crash)) {
-      auto optarg = arg->getValue();
-      m_option_data.AddInitialCommand(optarg, eCommandPlacementAfterCrash,
+      m_option_data.AddInitialCommand(arg_value, eCommandPlacementAfterCrash,
                                       false, error);
       if (error.Fail())
         return error;
     }
 
     if (arg->getOption().matches(OPT_source)) {
-      auto optarg = arg->getValue();
-      m_option_data.AddInitialCommand(optarg, eCommandPlacementAfterFile, true,
-                                      error);
+      m_option_data.AddInitialCommand(arg_value, eCommandPlacementAfterFile,
+                                      true, error);
       if (error.Fail())
         return error;
     }
 
     if (arg->getOption().matches(OPT_source_before_file)) {
-      auto optarg = arg->getValue();
-      m_option_data.AddInitialCommand(optarg, eCommandPlacementBeforeFile, true,
-                                      error);
+      m_option_data.AddInitialCommand(arg_value, eCommandPlacementBeforeFile,
+                                      true, error);
       if (error.Fail())
         return error;
     }
 
     if (arg->getOption().matches(OPT_one_line)) {
-      auto optarg = arg->getValue();
-      m_option_data.AddInitialCommand(optarg, eCommandPlacementAfterFile, false,
-                                      error);
+      m_option_data.AddInitialCommand(arg_value, eCommandPlacementAfterFile,
+                                      false, error);
       if (error.Fail())
         return error;
     }
 
     if (arg->getOption().matches(OPT_one_line_before_file)) {
-      auto optarg = arg->getValue();
-      m_option_data.AddInitialCommand(optarg, eCommandPlacementBeforeFile,
+      m_option_data.AddInitialCommand(arg_value, eCommandPlacementBeforeFile,
                                       false, error);
       if (error.Fail())
         return error;
index 60a9f44..90df963 100644 (file)
@@ -170,9 +170,10 @@ def: Flag<["-"], "h">,
   Alias<help>,
   HelpText<"Alias for --help">;
 
-def core: F<"core">,
-  HelpText<"Tells the debugger to use the full path to <core> as the core file.">;
-def: Flag<["-"], "c">,
+def core: Separate<["--", "-"], "core">,
+  MetaVarName<"<filename>">,
+  HelpText<"Tells the debugger to use the full path to <filename> as the core file.">;
+def: Separate<["-"], "c">,
   Alias<core>,
   HelpText<"Alias for --core">;