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

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");
         }
     }
 

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

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;