[trac3664] Updates per second review comments

Add comments to the test code

Modify the getopt code to explicitly handle unknown arguments and
missing option arguments.

diff --git a/src/bin/lfc/lfc_controller.cc b/src/bin/lfc/lfc_controller.cc
index c1bdc1334701f6d6482dfcb7bb38dec04d923e99..d18247898ef8c261d1c1da5fc443eab017b44302 100644 (file)
--- a/src/bin/lfc/lfc_controller.cc
+++ b/src/bin/lfc/lfc_controller.cc
@@ -55,7 +55,7 @@ LFCController::parseArgs(int argc, char* argv[]) {
 
     opterr = 0;
     optind = 1;
-    while ((ch = getopt(argc, argv, "46dvVp:i:o:c:f:")) != -1) {
+    while ((ch = getopt(argc, argv, ":46dvVp:i:o:c:f:")) != -1) {
         switch (ch) {
         case '4':
             // Process DHCPv4 lease files.
@@ -124,11 +124,20 @@ LFCController::parseArgs(int argc, char* argv[]) {
 
         case 'h':
             usage("");
-            return;
+            exit(EXIT_SUCCESS);
+
+        case '?':
+            // Unknown argument
+            isc_throw(InvalidUsage, "Unknown argument");
+
+        case ':':
+            // Missing option argument
+            isc_throw(InvalidUsage, "Missing option argument");
 
         default:
-            // We should never actually get here
-            isc_throw(InvalidUsage, "Illegal result.");
+            // I don't think we should get here as the unknown arguments
+            // and missing options cases should cover everything else
+            isc_throw(InvalidUsage, "Invalid command line");
         }
     }
 

diff --git a/src/bin/lfc/lfc_controller.h b/src/bin/lfc/lfc_controller.h
index b50710649b38fb0f350b8ee6ba2fd4596b6afdcc..84f42d7f9f2f4131d4d342f816cf7d854b82c5fd 100644 (file)
--- a/src/bin/lfc/lfc_controller.h
+++ b/src/bin/lfc/lfc_controller.h
@@ -35,7 +35,7 @@ public:
 /// invoke the code to process the lease files and finally to rename
 /// the lease files as necessary.
 ///
-/// @todo The current code simply processes the command line we still need
+/// @todo The current code simply processes the command line we still need to
 /// -# handle PID file manipulation
 /// -# invoke the code to read, process and write the lease files
 /// -# rename and delete the shell files as required

diff --git a/src/bin/lfc/tests/lfc_controller_unittests.cc b/src/bin/lfc/tests/lfc_controller_unittests.cc
index 198e926af42cb45940e3509badb17364da5ed047..163a1606b07e840f289c510301669e35e30d1f39 100644 (file)
--- a/src/bin/lfc/tests/lfc_controller_unittests.cc
+++ b/src/bin/lfc/tests/lfc_controller_unittests.cc
@@ -20,6 +20,9 @@ using namespace std;
 
 namespace {
 
+/// @brief Verify initial state of LFC controller.
+/// Create an instance of the controller and see that
+/// all of the initial values are empty as expected.
 TEST(LFCControllerTest, initialValues) {
     LFCController lfc_controller;
 
@@ -33,6 +36,9 @@ TEST(LFCControllerTest, initialValues) {
     EXPECT_TRUE(lfc_controller.getPidFile().empty());
 }
 
+/// @brief Verify that parsing a full command line works.
+/// Parse a complete command line then verify the parsed
+/// and saved data matches our expectations.
 TEST(LFCControllerTest, fullCommandLine) {
     LFCController lfc_controller;
 
@@ -62,6 +68,10 @@ TEST(LFCControllerTest, fullCommandLine) {
     EXPECT_EQ(lfc_controller.getFinishFile(), "finish");
 }
 
+/// @brief Verify that parsing a correct but incomplete line fails.
+/// Parse a command line that is correctly formatted but isn't complete
+/// (doesn't include some options or an some option arguments).  We
+/// expect that the parse will fail with an InvalidUsage exception.
 TEST(LFCControllerTest, notEnoughData) {
     LFCController lfc_controller;
 
@@ -92,7 +102,11 @@ TEST(LFCControllerTest, notEnoughData) {
     ASSERT_NO_THROW(lfc_controller.parseArgs(argc, argv));
 }
 
-
+/// @brief Verify that extra arguments cause the parse to fail.
+/// Parse a full command line plus some extra arguments on the end
+/// to verify that we don't stop parsing when we find all of the
+/// required arguments.  We exepct the parse to fail with an
+/// InvalidUsage exception.
 TEST(LFCControllerTest, tooMuchData) {
     LFCController lfc_controller;
 
@@ -120,6 +134,9 @@ TEST(LFCControllerTest, tooMuchData) {
     EXPECT_THROW(lfc_controller.parseArgs(argc, argv), InvalidUsage);
 }
 
+/// @brief Verify that unknown arguments cause the parse to fail.
+/// Parse some unknown arguments to verify that we generate the
+/// proper InvalidUsage exception.
 TEST(LFCControllerTest, someBadData) {
     LFCController lfc_controller;