[Lldb-commits] [lldb] r364317 - Options: Correctly check for missing arguments

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun 25 07:02:39 2019
New Revision: 364317

URL: http://llvm.org/viewvc/llvm-project?rev=364317&view=rev
Log:
Options: Correctly check for missing arguments

Relying on the value of optind for detecting missing arguments is
unreliable because its value after a failed parse is an implementation
detail. A more correct way to achieve this is to pass ':' at the
beginning of option string, which tells getopt to return ':' for missing
arguments.

For this to work, I also had to add a nullptr at the end of the argv
vector, as some getopt implementations did not work without that. This
is also an implementation detail, as getopt should normally be called
with argc+argc "as to main function" (i.e. null-terminated).

Thanks to Michał Górny for testing this patch out on NetBSD.

Modified:
    lldb/trunk/source/Interpreter/Options.cpp

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=364317&r1=364316&r2=364317&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Tue Jun 25 07:02:39 2019
@@ -1336,6 +1336,9 @@ llvm::Expected<Args> Options::Parse(cons
                                                llvm::inconvertibleErrorCode());
   }
 
+  // Leading : tells getopt to return a : for a missing option argument AND to
+  // suppress error messages.
+  sstr << ":";
   for (int i = 0; long_options[i].definition != nullptr; ++i) {
     if (long_options[i].flag == nullptr) {
       if (isprint8(long_options[i].val)) {
@@ -1357,8 +1360,7 @@ llvm::Expected<Args> Options::Parse(cons
   std::vector<char *> argv = GetArgvForParsing(args);
   // If the last option requires an argument but doesn't have one,
   // some implementations of getopt_long will still try to read it.
-  char overflow = 0;
-  argv.push_back(&overflow);
+  argv.push_back(nullptr);
   std::unique_lock<std::mutex> lock;
   OptionParser::Prepare(lock);
   int val;
@@ -1367,7 +1369,7 @@ llvm::Expected<Args> Options::Parse(cons
     val = OptionParser::Parse(argv.size() - 1, &*argv.begin(), sstr.GetString(),
                               long_options, &long_options_index);
 
-    if ((size_t)OptionParser::GetOptionIndex() > argv.size() - 1) {
+    if (val == ':') {
       error.SetErrorStringWithFormat("last option requires an argument");
       break;
     }