[#2788] exhaust options before throwing error

Prior to this change, if parseArgs() was called twice during the same
program lifetime and it stumbled on an unsupported option and throwed an
exception on the first call, the previous set of arguments lived on to
be parsed by the second call. This is a situation that likely arises
only in unit tests, but let us fix it properly to at least silence the unit
test failure on alpine, which was happening because of different
implementation of getopt from musl, and which motivated looking into how
getopt behaves. To make the bug evident even in a non-alpine environment, add an
EXPECT_THROW_MSG in DStubControllerTest.commandLineArgs when parsing argv3, and
see that it outputs "unsupported option: [s]" instead of
"extraneous command line information".

5 files changed, 24 insertions, 12 deletions

diff --git a/src/bin/agent/tests/ca_controller_unittests.cc b/src/bin/agent/tests/ca_controller_unittests.cc
index dc63e01a39..a20ea1a770 100644
--- a/src/bin/agent/tests/ca_controller_unittests.cc
+++ b/src/bin/agent/tests/ca_controller_unittests.cc
@@ -216,7 +216,7 @@ TEST_F(CtrlAgentControllerTest, commandLineArgs) {
     char* argv2[] = { const_cast<char*>("progName"),
                       const_cast<char*>("-x") };
     argc = 2;
-    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: [x] ");
+    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: -x");
 }
 
 // Tests application process creation and initialization.
diff --git a/src/bin/d2/tests/d2_controller_unittests.cc b/src/bin/d2/tests/d2_controller_unittests.cc
index 8bac486ba6..52e4cc0565 100644
--- a/src/bin/d2/tests/d2_controller_unittests.cc
+++ b/src/bin/d2/tests/d2_controller_unittests.cc
@@ -132,7 +132,7 @@ TEST_F(D2ControllerTest, commandLineArgs) {
     char* argv2[] = { const_cast<char*>("progName"),
                       const_cast<char*>("-x") };
     argc = 2;
-    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: [x] ");
+    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: -x");
 }
 
 /// @brief Tests application process creation and initialization.
diff --git a/src/bin/netconf/tests/netconf_controller_unittests.cc b/src/bin/netconf/tests/netconf_controller_unittests.cc
index 03002f7c4c..3bbd708f3d 100644
--- a/src/bin/netconf/tests/netconf_controller_unittests.cc
+++ b/src/bin/netconf/tests/netconf_controller_unittests.cc
@@ -131,7 +131,7 @@ TEST_F(NetconfControllerTest, commandLineArgs) {
     char* argv2[] = { const_cast<char*>("progName"),
                       const_cast<char*>("-x") };
     argc = 2;
-    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: [x] ");
+    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: -x");
 }
 
 // Tests application process creation and initialization.
diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc
index d6f14d1ea9..e8d72d5f2e 100644
--- a/src/lib/process/d_controller.cc
+++ b/src/lib/process/d_controller.cc
@@ -293,10 +293,16 @@ DControllerBase::parseArgs(int argc, char* argv[]) {
             break;
 
         case '?': {
+            char const saved_optopt(optopt);
+            std::string const saved_optarg(optarg ? optarg : std::string());
+
+            // Exhaust all remaining options in case parseArgs() is called again.
+            while (getopt(argc, argv, opts.c_str()) != -1) {
+            }
+
             // We hit an invalid option.
-            isc_throw(InvalidUsage, "unsupported option: ["
-                      << static_cast<char>(optopt) << "] "
-                      << (!optarg ? "" : optarg));
+            isc_throw(InvalidUsage, "unsupported option: -" << saved_optopt <<
+                      (saved_optarg.empty() ? std::string() : " " + saved_optarg));
 
             break;
             }
@@ -304,10 +310,16 @@ DControllerBase::parseArgs(int argc, char* argv[]) {
         default:
             // We hit a valid custom option
             if (!customOption(ch, optarg)) {
-                // This would be a programmatic error.
-                isc_throw(InvalidUsage, " Option listed but implemented?: ["
-                          << static_cast<char>(ch) << "] "
-                          << (!optarg ? "" : optarg));
+                char const saved_optopt(optopt);
+                std::string const saved_optarg(optarg ? optarg : std::string());
+
+                // Exhaust all remaining options in case parseArgs() is called again.
+                while (getopt(argc, argv, opts.c_str()) != -1) {
+                }
+
+                // We hit an invalid option.
+                isc_throw(InvalidUsage, "unsupported option: -" << saved_optopt <<
+                        (saved_optarg.empty() ? std::string() : " " + saved_optarg));
             }
             break;
         }
diff --git a/src/lib/process/tests/d_controller_unittests.cc b/src/lib/process/tests/d_controller_unittests.cc
index 1063e162aa..d44b46459f 100644
--- a/src/lib/process/tests/d_controller_unittests.cc
+++ b/src/lib/process/tests/d_controller_unittests.cc
@@ -99,14 +99,14 @@ TEST_F(DStubControllerTest, commandLineArgs) {
     char* argv2[] = { const_cast<char*>("progName"),
                       const_cast<char*>("-bs") };
     argc = 2;
-    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: [b] ");
+    EXPECT_THROW_MSG(parseArgs(argc, argv2), InvalidUsage, "unsupported option: -b");
 
     // Verify that extraneous information is detected.
     char* argv3[] = { const_cast<char*>("progName"),
                       const_cast<char*>("extra"),
                       const_cast<char*>("information") };
     argc = 3;
-    EXPECT_THROW_MSG(parseArgs(argc, argv3), InvalidUsage, "unsupported option: [s] ");
+    EXPECT_THROW_MSG(parseArgs(argc, argv3), InvalidUsage, "extraneous command line information");
 }
 
 /// @brief Tests application process creation and initialization.