[#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".

diff --git a/src/bin/agent/tests/ca_controller_unittests.cc b/src/bin/agent/tests/ca_controller_unittests.cc
index dc63e01a39da28813d9daddfb69ca1fe8de69b00..a20ea1a770ad098a172572101645cd4f8b7a6670 100644 (file)
--- 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 8bac486ba6ec465ea4f7ef8acbb90da0ca16de2f..52e4cc0565206666e2e2902049518bf43881c0ec 100644 (file)
--- 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 03002f7c4cad57035d1c5191525a5082d4b77c5d..3bbd708f3d26022334d7c7daf62efc657c7ed48e 100644 (file)
--- 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 d6f14d1ea92717853efa2de7b97aa8a9d715082e..e8d72d5f2e1015c536968e81f92215b57c8d7ffc 100644 (file)
--- 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 1063e162aa9373664b8bc455f49d49e21ee564d4..d44b46459f675276f57c8e1e8b6af14200e3d0ec 100644 (file)
--- 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.