argument: Support multiple arguments

This will be used in future patches. Right now this should retain
similar behavior to the current argument parsing semantics. The
difference is we now require arguments to be specified only once and
error if they are specified multiple times.

Change-Id: I21e4cf9734f045c2b0991f7ed0ec6e6a569eec7d
Signed-off-by: William A. Kennington III <wak@google.com>

4 files changed, 54 insertions, 34 deletions

diff --git a/argument.cpp b/argument.cpp
index ed74a9d..edbaa65 100644
--- a/argument.cpp
+++ b/argument.cpp
@@ -25,8 +25,8 @@ namespace watchdog
 
 using namespace std::string_literals;
 
+const std::vector<std::string> emptyArg;
 const std::string ArgumentParser::trueString = "true"s;
-const std::string ArgumentParser::emptyString = ""s;
 
 const char* ArgumentParser::optionStr = "p:s:t:ch";
 const option ArgumentParser::options[] =
@@ -41,7 +41,8 @@ const option ArgumentParser::options[] =
 
 ArgumentParser::ArgumentParser(int argc, char * const argv[])
 {
-    int option = 0;
+    int option;
+    int opt_idx = 0;
 
     // We have to reset optind because getopt_long keeps global state
     // and uses optind to track what argv index it needs to process next.
@@ -49,33 +50,35 @@ ArgumentParser::ArgumentParser(int argc, char * const argv[])
     // already process instructions, optind may not be initialized to point to
     // the beginning of our argv.
     optind = 0;
-    while (-1 != (option = getopt_long(argc, argv, optionStr, options, nullptr)))
+    while (-1 != (option = getopt_long(argc, argv, optionStr, options, &opt_idx)))
     {
-        if ((option == '?') || (option == 'h'))
+        if (option == '?' || option == 'h')
         {
             usage(argv);
             exit(-1);
         }
 
-        auto i = &options[0];
-        while ((i->val != option) && (i->val != 0))
+        auto i = &options[opt_idx];
+        while (i->val && i->val != option)
         {
             ++i;
         }
 
         if (i->val)
         {
-            arguments[i->name] = (optarg ? optarg : trueString);
+            arguments[i->name].push_back(optarg ? optarg : trueString);
         }
+
+        opt_idx = 0;
     }
 }
 
-const std::string& ArgumentParser::operator[](const std::string& opt)
+const std::vector<std::string>& ArgumentParser::operator[](const std::string& opt)
 {
     auto i = arguments.find(opt);
     if (i == arguments.end())
     {
-        return emptyString;
+        return emptyArg;
     }
     else
     {
diff --git a/argument.hpp b/argument.hpp
index ee9e8c8..1c07c5c 100644
--- a/argument.hpp
+++ b/argument.hpp
@@ -3,6 +3,7 @@
 #include <getopt.h>
 #include <map>
 #include <string>
+#include <vector>
 
 namespace phosphor
 {
@@ -35,7 +36,7 @@ class ArgumentParser
          *
          *  @return argument which is a standard optarg
          */
-        const std::string& operator[](const std::string& opt);
+        const std::vector<std::string>& operator[](const std::string& opt);
 
         /** @brief Displays usage
          *
@@ -46,12 +47,9 @@ class ArgumentParser
         /** @brief Set to 'true' when an option is passed */
         static const std::string trueString;
 
-        /** @brief Set to '' when an option is not passed */
-        static const std::string emptyString;
-
     private:
         /** @brief Option to argument mapping */
-        std::map<const std::string, std::string> arguments;
+        std::map<const std::string, std::vector<std::string> > arguments;
 
         /** @brief Array of struct options as needed by getopt_long */
         static const option options[];
diff --git a/mainapp.cpp b/mainapp.cpp
index 2469363..5819fb4 100644
--- a/mainapp.cpp
+++ b/mainapp.cpp
@@ -41,29 +41,44 @@ int main(int argc, char** argv)
     auto continueParam = (options)["continue"];
     // Default it to exit on watchdog timeout
     auto continueAfterTimeout = false;
-    if (continueParam == phosphor::watchdog::ArgumentParser::trueString)
+    if (!continueParam.empty())
     {
         continueAfterTimeout = true;
     }
 
     // Parse out path argument.
-    auto path = (options)["path"];
-    if (path == phosphor::watchdog::ArgumentParser::emptyString)
+    auto pathParam = (options)["path"];
+    if (pathParam.empty())
     {
         exitWithError("Path not specified.", argv);
     }
+    if (pathParam.size() > 1)
+    {
+        exitWithError("Multiple paths specified.", argv);
+    }
+    auto path = pathParam.back();
 
     // Parse out service name argument
-    auto service = (options)["service"];
-    if (service == phosphor::watchdog::ArgumentParser::emptyString)
+    auto serviceParam = (options)["service"];
+    if (serviceParam.empty())
     {
         exitWithError("Service not specified.", argv);
     }
+    if (serviceParam.size() > 1)
+    {
+        exitWithError("Multiple services specified.", argv);
+    }
+    auto service = serviceParam.back();
 
     // Parse out target argument. It is fine if the caller does not
     // pass this if they are not interested in calling into any target
     // on meeting a condition.
-    auto target = (options)["target"];
+    auto targetParam = (options)["target"];
+    if (targetParam.size() > 1)
+    {
+        exitWithError("Multiple targets specified.", argv);
+    }
+    auto target = targetParam.back();
 
     sd_event* event = nullptr;
     auto r = sd_event_default(&event);
diff --git a/test/argument_test.cpp b/test/argument_test.cpp
index 79b936f..e293e99 100644
--- a/test/argument_test.cpp
+++ b/test/argument_test.cpp
@@ -1,10 +1,12 @@
 #include <string>
+#include <vector>
 
 #include "argument.hpp"
 #include "argument_test.hpp"
 
 static const std::string expected_path1 = "/arg1-test-path";
 static const std::string expected_target1 = "t1.target";
+static const std::string expected_target2 = "t2.target";
 
 // Allow for a single unrecognized option then the Usage printout
 static const std::string invalid_arg_regex =
@@ -29,9 +31,9 @@ TEST_F(ArgumentTest, NoOptions)
         &arg0[0], nullptr
     };
     ArgumentParser ap(sizeof(args)/sizeof(char *) - 1, args);
-    EXPECT_EQ(ArgumentParser::emptyString, ap["path"]);
-    EXPECT_EQ(ArgumentParser::emptyString, ap["continue"]);
-    EXPECT_EQ(ArgumentParser::emptyString, ap["arbitrary_unknown"]);
+    EXPECT_EQ(std::vector<std::string>({}), ap["path"]);
+    EXPECT_EQ(std::vector<std::string>({}), ap["continue"]);
+    EXPECT_EQ(std::vector<std::string>({}), ap["arbitrary_unknown"]);
 }
 
 /** @brief ArgumentParser should return true for an existing no-arg option
@@ -46,8 +48,9 @@ TEST_F(ArgumentTest, LongOptionNoArg)
         &arg0[0], &arg_continue[0], &arg_extra[0], nullptr
     };
     ArgumentParser ap(sizeof(args)/sizeof(char *) - 1, args);
-    EXPECT_EQ(ArgumentParser::emptyString, ap["path"]);
-    EXPECT_EQ(ArgumentParser::trueString, ap["continue"]);
+    EXPECT_EQ(std::vector<std::string>({}), ap["path"]);
+    EXPECT_EQ(std::vector<std::string>({ArgumentParser::trueString}),
+            ap["continue"]);
 }
 
 /** @brief ArgumentParser should return a string for long options that
@@ -62,7 +65,7 @@ TEST_F(ArgumentTest, LongOptionRequiredArg)
         &arg0[0], &arg_path[0], &arg_path_val[0], &arg_extra[0], nullptr
     };
     ArgumentParser ap(sizeof(args)/sizeof(char *) - 1, args);
-    EXPECT_EQ(expected_path1, ap["path"]);
+    EXPECT_EQ(std::vector<std::string>({expected_path1}), ap["path"]);
 }
 
 /** @brief ArgumentParser should return a string for long options that
@@ -76,7 +79,7 @@ TEST_F(ArgumentTest, LongOptionInlineArg)
         &arg0[0], &arg_path[0], &arg_extra[0], nullptr
     };
     ArgumentParser ap(sizeof(args)/sizeof(char *) - 1, args);
-    EXPECT_EQ(expected_path1, ap["path"]);
+    EXPECT_EQ(std::vector<std::string>({expected_path1}), ap["path"]);
 }
 
 /** @brief ArgumentParser should return a string for short options that
@@ -91,12 +94,10 @@ TEST_F(ArgumentTest, ShortOptionRequiredArg)
         &arg0[0], &arg_path[0], &arg_path_val[0], &arg_extra[0], nullptr
     };
     ArgumentParser ap(sizeof(args)/sizeof(char *) - 1, args);
-    EXPECT_EQ(expected_path1, ap["path"]);
+    EXPECT_EQ(std::vector<std::string>({expected_path1}), ap["path"]);
 }
 
 /** @brief ArgumentParser should be able to handle parsing multiple options
- *         Make sure that when passed multiple of the same option it uses
- *         the argument to the option passed last
  *         Make sure this works for no-arg and required-arg type options
  *         Make sure this works between short and long options
  */
@@ -105,7 +106,7 @@ TEST_F(ArgumentTest, MultiOptionOverride)
     std::string arg_continue_short = "-c";
     std::string arg_path = "--path=" + expected_path1;
     std::string arg_continue_long = "--continue";
-    std::string arg_target = "--target=/unused-path";
+    std::string arg_target = "--target=" + expected_target2;
     std::string arg_target_short = "-t";
     std::string arg_target_val = expected_target1;
     char * const args[] = {
@@ -113,9 +114,12 @@ TEST_F(ArgumentTest, MultiOptionOverride)
         &arg_target[0], &arg_target_short[0], &arg_target_val[0], nullptr
     };
     ArgumentParser ap(sizeof(args)/sizeof(char *) - 1, args);
-    EXPECT_EQ(expected_path1, ap["path"]);
-    EXPECT_EQ(ArgumentParser::trueString, ap["continue"]);
-    EXPECT_EQ(expected_target1, ap["target"]);
+    EXPECT_EQ(std::vector<std::string>({expected_path1}), ap["path"]);
+    EXPECT_EQ(std::vector<std::string>({
+                ArgumentParser::trueString, ArgumentParser::trueString}),
+            ap["continue"]);
+    EXPECT_EQ(std::vector<std::string>({expected_target2, expected_target1}),
+            ap["target"]);
 }
 
 /** @brief ArgumentParser should print usage information when given a help