[{"id":734,"web_url":"https://patchwork.libcamera.org/comment/734/","msgid":"<20190201093544.GN19527@bigcity.dyn.berto.se>","date":"2019-02-01T09:35:44","subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: options: Add option type\n\thandling to options parser","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nNeat patch!\n\nOn 2019-02-01 01:47:19 +0200, Laurent Pinchart wrote:\n> Extend the options parser with support for option types. All options\n> must now specify the type of their argument, and the parser\n> automatically parses the argument and handles errors internally.\n> Available types are none, integer or string.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/cam/main.cpp    |  10 ++--\n>  src/cam/options.cpp | 133 ++++++++++++++++++++++++++++++++++++++++++--\n>  src/cam/options.h   |  41 +++++++++++++-\n>  3 files changed, 172 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index bde47a8f1798..7934d0bf4132 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -37,10 +37,12 @@ static int parseOptions(int argc, char *argv[])\n>  {\n>  \tOptionsParser parser;\n>  \n> -\tparser.addOption(OptCamera, \"Specify which camera to operate on\",\n> -\t\t\t \"camera\", ArgumentRequired, \"camera\");\n> -\tparser.addOption(OptHelp, \"Display this help message\", \"help\");\n> -\tparser.addOption(OptList, \"List all cameras\", \"list\");\n> +\tparser.addOption(OptCamera, OptionString,\n> +\t\t\t \"Specify which camera to operate on\", \"camera\",\n> +\t\t\t ArgumentRequired, \"camera\");\n> +\tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> +\t\t\t \"help\");\n> +\tparser.addOption(OptList, OptionNone, \"List all cameras\", \"list\");\n>  \n>  \toptions = parser.parse(argc, argv);\n>  \tif (!options.valid())\n> diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> index c13022ce1b84..204081f3cd8e 100644\n> --- a/src/cam/options.cpp\n> +++ b/src/cam/options.cpp\n> @@ -12,6 +12,30 @@\n>  \n>  #include \"options.h\"\n>  \n> +/* -----------------------------------------------------------------------------\n> + * Option\n> + */\n> +\n> +const char *Option::typeName() const\n> +{\n> +\tswitch (type) {\n> +\tcase OptionNone:\n> +\t\treturn \"none\";\n> +\n> +\tcase OptionInteger:\n> +\t\treturn \"integer\";\n> +\n> +\tcase OptionString:\n> +\t\treturn \"string\";\n> +\t}\n> +\n> +\treturn \"unknown\";\n> +}\n> +\n> +/* -----------------------------------------------------------------------------\n> + * OptionBase<T>\n> + */\n> +\n>  template <typename T>\n>  bool OptionsBase<T>::valid() const\n>  {\n> @@ -25,11 +49,45 @@ bool OptionsBase<T>::isSet(const T &opt) const\n>  }\n>  \n>  template <typename T>\n> -const std::string &OptionsBase<T>::operator[](const T &opt) const\n> +const OptionValue &OptionsBase<T>::operator[](const T &opt) const\n>  {\n>  \treturn values_.find(opt)->second;\n>  }\n>  \n> +template <typename T>\n> +bool OptionsBase<T>::parseValue(const T &opt, const Option &option,\n> +\t\t\t\tconst char *optarg)\n> +{\n> +\tOptionValue value;\n> +\n> +\tswitch (option.type) {\n> +\tcase OptionNone:\n> +\t\tbreak;\n> +\n> +\tcase OptionInteger:\n> +\t\tunsigned int integer;\n> +\n> +\t\tif (optarg) {\n> +\t\t\tchar *endptr;\n> +\t\t\tinteger = strtoul(optarg, &endptr, 10);\n> +\t\t\tif (*endptr != '\\0')\n> +\t\t\t\treturn false;\n> +\t\t} else {\n> +\t\t\tinteger = 0;\n> +\t\t}\n> +\n> +\t\tvalue = OptionValue(integer);\n> +\t\tbreak;\n> +\n> +\tcase OptionString:\n> +\t\tvalue = OptionValue(optarg ? optarg : \"\");\n> +\t\tbreak;\n> +\t}\n> +\n> +\tvalues_[opt] = value;\n> +\treturn true;\n> +}\n> +\n>  template <typename T>\n>  void OptionsBase<T>::clear()\n>  {\n> @@ -38,8 +96,53 @@ void OptionsBase<T>::clear()\n>  \n>  template class OptionsBase<int>;\n>  \n> -bool OptionsParser::addOption(int opt, const char *help, const char *name,\n> -\t\t\t      OptionArgument argument, const char *argumentName)\n> +/* -----------------------------------------------------------------------------\n> + * OptionValue\n> + */\n> +\n> +OptionValue::OptionValue()\n> +\t: type_(OptionNone)\n> +{\n> +}\n> +\n> +OptionValue::OptionValue(int value)\n> +\t: type_(OptionInteger), integer_(value)\n> +{\n> +}\n> +\n> +OptionValue::OptionValue(const char *value)\n> +\t: type_(OptionString), string_(value)\n> +{\n> +}\n> +\n> +OptionValue::OptionValue(const std::string &value)\n> +\t: type_(OptionString), string_(value)\n> +{\n> +}\n> +\n> +OptionValue::operator int() const\n> +{\n> +\tif (type_ != OptionInteger)\n> +\t\treturn 0;\n> +\n> +\treturn integer_;\n> +}\n> +\n> +OptionValue::operator std::string() const\n> +{\n> +\tif (type_ != OptionString)\n> +\t\treturn std::string();\n> +\n> +\treturn string_;\n> +}\n> +\n> +/* -----------------------------------------------------------------------------\n> + * OptionsParser\n> + */\n> +\n> +bool OptionsParser::addOption(int opt, OptionType type, const char *help,\n> +\t\t\t      const char *name, OptionArgument argument,\n> +\t\t\t      const char *argumentName)\n>  {\n>  \t/*\n>  \t * Options must have at least a short or long name, and a text message.\n> @@ -56,7 +159,8 @@ bool OptionsParser::addOption(int opt, const char *help, const char *name,\n>  \tif (optionsMap_.find(opt) != optionsMap_.end())\n>  \t\treturn false;\n>  \n> -\toptions_.push_back(Option({ opt, name, argument, argumentName, help }));\n> +\toptions_.push_back(Option({ opt, type, name, argument, argumentName,\n> +\t\t\t\t    help }));\n>  \toptionsMap_[opt] = &options_.back();\n>  \treturn true;\n>  }\n> @@ -126,7 +230,13 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)\n>  \t\t\tbreak;\n>  \t\t}\n>  \n> -\t\toptions.values_[c] = optarg ? optarg : \"\";\n> +\t\tconst Option &option = *optionsMap_[c];\n> +\t\tif (!options.parseValue(c, option, optarg)) {\n> +\t\t\tparseValueError(option);\n> +\t\t\tusage();\n> +\t\t\toptions.clear();\n> +\t\t\tbreak;\n> +\t\t}\n>  \t}\n>  \n>  \treturn options;\n> @@ -193,3 +303,16 @@ void OptionsParser::usage()\n>  \t\t}\n>  \t}\n>  }\n> +\n> +void OptionsParser::parseValueError(const Option &option)\n> +{\n> +\tstd::string optionName;\n> +\n> +\tif (option.name)\n> +\t\toptionName = \"--\" + std::string(option.name);\n> +\telse\n> +\t\toptionName = \"-\" + static_cast<char>(option.opt);\n> +\n> +\tstd::cerr << \"Can't parse \" << option.typeName()\n> +\t\t  << \" argument for option \" << optionName << std::endl;\n> +}\n> diff --git a/src/cam/options.h b/src/cam/options.h\n> index b9b7bd258c03..8b611d374fd5 100644\n> --- a/src/cam/options.h\n> +++ b/src/cam/options.h\n> @@ -17,8 +17,15 @@ enum OptionArgument {\n>  \tArgumentOptional,\n>  };\n>  \n> +enum OptionType {\n> +\tOptionNone,\n> +\tOptionInteger,\n> +\tOptionString,\n> +};\n> +\n>  struct Option {\n>  \tint opt;\n> +\tOptionType type;\n>  \tconst char *name;\n>  \tOptionArgument argument;\n>  \tconst char *argumentName;\n> @@ -26,20 +33,45 @@ struct Option {\n>  \n>  \tbool hasShortOption() const { return isalnum(opt); }\n>  \tbool hasLongOption() const { return name != nullptr; }\n> +\tconst char *typeName() const;\n>  };\n>  \n> +class OptionValue;\n> +\n>  template <typename T>\n>  class OptionsBase\n>  {\n>  public:\n>  \tbool valid() const;\n>  \tbool isSet(const T &opt) const;\n> -\tconst std::string &operator[](const T &opt) const;\n> +\tconst OptionValue &operator[](const T &opt) const;\n>  \n>  private:\n>  \tfriend class OptionsParser;\n> -\tstd::map<T, std::string> values_;\n> +\n> +\tbool parseValue(const T &opt, const Option &option, const char *value);\n>  \tvoid clear();\n> +\n> +\tstd::map<T, OptionValue> values_;\n> +};\n> +\n> +class OptionValue\n> +{\n> +public:\n> +\tOptionValue();\n> +\tOptionValue(int value);\n> +\tOptionValue(const char *value);\n> +\tOptionValue(const std::string &value);\n> +\n> +\tOptionType type() const { return type_; }\n> +\n> +\toperator int() const;\n> +\toperator std::string() const;\n> +\n> +private:\n> +\tOptionType type_;\n> +\tint integer_;\n> +\tstd::string string_;\n>  };\n>  \n>  class OptionsParser\n> @@ -49,7 +81,8 @@ public:\n>  \t{\n>  \t};\n>  \n> -\tbool addOption(int opt, const char *help, const char *name = nullptr,\n> +\tbool addOption(int opt, OptionType type, const char *help,\n> +\t\t       const char *name = nullptr,\n>  \t\t       OptionArgument argument = ArgumentNone,\n>  \t\t       const char *argumentName = nullptr);\n>  \n> @@ -57,6 +90,8 @@ public:\n>  \tvoid usage();\n>  \n>  private:\n> +\tvoid parseValueError(const Option &option);\n> +\n>  \tstd::list<Option> options_;\n>  \tstd::map<unsigned int, Option *> optionsMap_;\n>  };\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7DFAE60C6A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  1 Feb 2019 10:35:48 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id a8so4552333lfk.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 01 Feb 2019 01:35:48 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tu15-v6sm1305766lja.63.2019.02.01.01.35.44\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 01 Feb 2019 01:35:45 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=b8VBkwpq6MhDn7dIDlCqx90zOMWtN7Q1yI7lZCNkD38=;\n\tb=Psei1yUXfft/9Tkfme2oskdZ/1UlAL9AcxA/8wHlaocljDwXW7UPk2QmTZjKnMbo9K\n\tufhcJP4vD76TLwz/VN7qsgmfqBEaKwzv6y9XUnjhV0ZsWEb9I/nwIVzljsu/8bnkV06P\n\tUQp9UQYyNS0uh8jqHij8M/qG2XAikkASEd3kBolJ+xkuBTbm465B4Sm56uwEQ+nN8vgW\n\t+oNIGHw7gg6kh+JC6kwlMUyYJYSx+4ZMIl2mYLt2bCKSw4Z7thgUDeoIYJvfT1S55CYo\n\tADXPGQKLp4WJRb5pFB+BXo0S5Sy6LAS+pWaeciyb7G5N48LWU25iP46rlQhCv0xWRp3G\n\tiF9w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=b8VBkwpq6MhDn7dIDlCqx90zOMWtN7Q1yI7lZCNkD38=;\n\tb=hfsx8rfDSukvao0Fk8M5VMN1Ge0c00Qyg48iGwQ3045a1xp16MWywdbLwUf4VGwfU7\n\t2H5VIRvSX0XHPkvTIHdZoc7tq6epAsWDYauUIwVB6TjmiG/O8TgHyGpIt5MQmWrjEYJp\n\t8CZAS2ytHJSF3fJIiZepkBIcxOuupm5TC7RZVRV55HWPj6+lohG3EVdA78VJygVfCyV+\n\t/nWapvMzfwDwr4Stlsh8GZvwlAw6Uiz0cW0HWlZ/ViodUtX3hqbNZYMLyyosl7TnvilN\n\tEKM8iiHLXidOdoRunB2htzP7G1INIk0f2zWXlAFUplPTql2IwQY5MQWQpOAx2V+EWH6S\n\tdEDA==","X-Gm-Message-State":"AJcUukdDyfEmh3Amheu1L0MEJkr2Jq+COiuYcPjC6xaaG2IMdm09JS6Q\n\tf7JAqOIHz0Qc3ipZ4ocUtlvSPOMUNQg=","X-Google-Smtp-Source":"ALg8bN4Blwi7j/V6ukx3T+db/xW94BhboBK3TEfLpAX36nCM/X5P/X2ossS5rwcGHMfYVjGvFtsEnA==","X-Received":"by 2002:a19:9a8c:: with SMTP id\n\tc134mr29637548lfe.152.1549013746098; \n\tFri, 01 Feb 2019 01:35:46 -0800 (PST)","Date":"Fri, 1 Feb 2019 10:35:44 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190201093544.GN19527@bigcity.dyn.berto.se>","References":"<20190131234721.22606-1-laurent.pinchart@ideasonboard.com>\n\t<20190131234721.22606-7-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190131234721.22606-7-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: options: Add option type\n\thandling to options parser","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 01 Feb 2019 09:35:48 -0000"}}]