[libcamera-devel,v2,6/8] cam: options: Add option type handling to options parser
diff mbox series

Message ID 20190131234721.22606-7-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • cam: add --format option to configure a stream
Related show

Commit Message

Laurent Pinchart Jan. 31, 2019, 11:47 p.m. UTC 
Extend the options parser with support for option types. All options
must now specify the type of their argument, and the parser
automatically parses the argument and handles errors internally.
Available types are none, integer or string.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/cam/main.cpp    |  10 ++--
 src/cam/options.cpp | 133 ++++++++++++++++++++++++++++++++++++++++++--
 src/cam/options.h   |  41 +++++++++++++-
 3 files changed, 172 insertions(+), 12 deletions(-)

Comments

Niklas Söderlund Feb. 1, 2019, 9:35 a.m. UTC | #1 
Hi Laurent,

Neat patch!

On 2019-02-01 01:47:19 +0200, Laurent Pinchart wrote:
> Extend the options parser with support for option types. All options
> must now specify the type of their argument, and the parser
> automatically parses the argument and handles errors internally.
> Available types are none, integer or string.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/cam/main.cpp    |  10 ++--
>  src/cam/options.cpp | 133 ++++++++++++++++++++++++++++++++++++++++++--
>  src/cam/options.h   |  41 +++++++++++++-
>  3 files changed, 172 insertions(+), 12 deletions(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index bde47a8f1798..7934d0bf4132 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -37,10 +37,12 @@ static int parseOptions(int argc, char *argv[])
>  {
>  	OptionsParser parser;
>  
> -	parser.addOption(OptCamera, "Specify which camera to operate on",
> -			 "camera", ArgumentRequired, "camera");
> -	parser.addOption(OptHelp, "Display this help message", "help");
> -	parser.addOption(OptList, "List all cameras", "list");
> +	parser.addOption(OptCamera, OptionString,
> +			 "Specify which camera to operate on", "camera",
> +			 ArgumentRequired, "camera");
> +	parser.addOption(OptHelp, OptionNone, "Display this help message",
> +			 "help");
> +	parser.addOption(OptList, OptionNone, "List all cameras", "list");
>  
>  	options = parser.parse(argc, argv);
>  	if (!options.valid())
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index c13022ce1b84..204081f3cd8e 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -12,6 +12,30 @@
>  
>  #include "options.h"
>  
> +/* -----------------------------------------------------------------------------
> + * Option
> + */
> +
> +const char *Option::typeName() const
> +{
> +	switch (type) {
> +	case OptionNone:
> +		return "none";
> +
> +	case OptionInteger:
> +		return "integer";
> +
> +	case OptionString:
> +		return "string";
> +	}
> +
> +	return "unknown";
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * OptionBase<T>
> + */
> +
>  template <typename T>
>  bool OptionsBase<T>::valid() const
>  {
> @@ -25,11 +49,45 @@ bool OptionsBase<T>::isSet(const T &opt) const
>  }
>  
>  template <typename T>
> -const std::string &OptionsBase<T>::operator[](const T &opt) const
> +const OptionValue &OptionsBase<T>::operator[](const T &opt) const
>  {
>  	return values_.find(opt)->second;
>  }
>  
> +template <typename T>
> +bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
> +				const char *optarg)
> +{
> +	OptionValue value;
> +
> +	switch (option.type) {
> +	case OptionNone:
> +		break;
> +
> +	case OptionInteger:
> +		unsigned int integer;
> +
> +		if (optarg) {
> +			char *endptr;
> +			integer = strtoul(optarg, &endptr, 10);
> +			if (*endptr != '\0')
> +				return false;
> +		} else {
> +			integer = 0;
> +		}
> +
> +		value = OptionValue(integer);
> +		break;
> +
> +	case OptionString:
> +		value = OptionValue(optarg ? optarg : "");
> +		break;
> +	}
> +
> +	values_[opt] = value;
> +	return true;
> +}
> +
>  template <typename T>
>  void OptionsBase<T>::clear()
>  {
> @@ -38,8 +96,53 @@ void OptionsBase<T>::clear()
>  
>  template class OptionsBase<int>;
>  
> -bool OptionsParser::addOption(int opt, const char *help, const char *name,
> -			      OptionArgument argument, const char *argumentName)
> +/* -----------------------------------------------------------------------------
> + * OptionValue
> + */
> +
> +OptionValue::OptionValue()
> +	: type_(OptionNone)
> +{
> +}
> +
> +OptionValue::OptionValue(int value)
> +	: type_(OptionInteger), integer_(value)
> +{
> +}
> +
> +OptionValue::OptionValue(const char *value)
> +	: type_(OptionString), string_(value)
> +{
> +}
> +
> +OptionValue::OptionValue(const std::string &value)
> +	: type_(OptionString), string_(value)
> +{
> +}
> +
> +OptionValue::operator int() const
> +{
> +	if (type_ != OptionInteger)
> +		return 0;
> +
> +	return integer_;
> +}
> +
> +OptionValue::operator std::string() const
> +{
> +	if (type_ != OptionString)
> +		return std::string();
> +
> +	return string_;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * OptionsParser
> + */
> +
> +bool OptionsParser::addOption(int opt, OptionType type, const char *help,
> +			      const char *name, OptionArgument argument,
> +			      const char *argumentName)
>  {
>  	/*
>  	 * Options must have at least a short or long name, and a text message.
> @@ -56,7 +159,8 @@ bool OptionsParser::addOption(int opt, const char *help, const char *name,
>  	if (optionsMap_.find(opt) != optionsMap_.end())
>  		return false;
>  
> -	options_.push_back(Option({ opt, name, argument, argumentName, help }));
> +	options_.push_back(Option({ opt, type, name, argument, argumentName,
> +				    help }));
>  	optionsMap_[opt] = &options_.back();
>  	return true;
>  }
> @@ -126,7 +230,13 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
>  			break;
>  		}
>  
> -		options.values_[c] = optarg ? optarg : "";
> +		const Option &option = *optionsMap_[c];
> +		if (!options.parseValue(c, option, optarg)) {
> +			parseValueError(option);
> +			usage();
> +			options.clear();
> +			break;
> +		}
>  	}
>  
>  	return options;
> @@ -193,3 +303,16 @@ void OptionsParser::usage()
>  		}
>  	}
>  }
> +
> +void OptionsParser::parseValueError(const Option &option)
> +{
> +	std::string optionName;
> +
> +	if (option.name)
> +		optionName = "--" + std::string(option.name);
> +	else
> +		optionName = "-" + static_cast<char>(option.opt);
> +
> +	std::cerr << "Can't parse " << option.typeName()
> +		  << " argument for option " << optionName << std::endl;
> +}
> diff --git a/src/cam/options.h b/src/cam/options.h
> index b9b7bd258c03..8b611d374fd5 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -17,8 +17,15 @@ enum OptionArgument {
>  	ArgumentOptional,
>  };
>  
> +enum OptionType {
> +	OptionNone,
> +	OptionInteger,
> +	OptionString,
> +};
> +
>  struct Option {
>  	int opt;
> +	OptionType type;
>  	const char *name;
>  	OptionArgument argument;
>  	const char *argumentName;
> @@ -26,20 +33,45 @@ struct Option {
>  
>  	bool hasShortOption() const { return isalnum(opt); }
>  	bool hasLongOption() const { return name != nullptr; }
> +	const char *typeName() const;
>  };
>  
> +class OptionValue;
> +
>  template <typename T>
>  class OptionsBase
>  {
>  public:
>  	bool valid() const;
>  	bool isSet(const T &opt) const;
> -	const std::string &operator[](const T &opt) const;
> +	const OptionValue &operator[](const T &opt) const;
>  
>  private:
>  	friend class OptionsParser;
> -	std::map<T, std::string> values_;
> +
> +	bool parseValue(const T &opt, const Option &option, const char *value);
>  	void clear();
> +
> +	std::map<T, OptionValue> values_;
> +};
> +
> +class OptionValue
> +{
> +public:
> +	OptionValue();
> +	OptionValue(int value);
> +	OptionValue(const char *value);
> +	OptionValue(const std::string &value);
> +
> +	OptionType type() const { return type_; }
> +
> +	operator int() const;
> +	operator std::string() const;
> +
> +private:
> +	OptionType type_;
> +	int integer_;
> +	std::string string_;
>  };
>  
>  class OptionsParser
> @@ -49,7 +81,8 @@ public:
>  	{
>  	};
>  
> -	bool addOption(int opt, const char *help, const char *name = nullptr,
> +	bool addOption(int opt, OptionType type, const char *help,
> +		       const char *name = nullptr,
>  		       OptionArgument argument = ArgumentNone,
>  		       const char *argumentName = nullptr);
>  
> @@ -57,6 +90,8 @@ public:
>  	void usage();
>  
>  private:
> +	void parseValueError(const Option &option);
> +
>  	std::list<Option> options_;
>  	std::map<unsigned int, Option *> optionsMap_;
>  };
> -- 
> Regards,
> 
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index bde47a8f1798..7934d0bf4132 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -37,10 +37,12 @@  static int parseOptions(int argc, char *argv[])
 {
 	OptionsParser parser;
 
-	parser.addOption(OptCamera, "Specify which camera to operate on",
-			 "camera", ArgumentRequired, "camera");
-	parser.addOption(OptHelp, "Display this help message", "help");
-	parser.addOption(OptList, "List all cameras", "list");
+	parser.addOption(OptCamera, OptionString,
+			 "Specify which camera to operate on", "camera",
+			 ArgumentRequired, "camera");
+	parser.addOption(OptHelp, OptionNone, "Display this help message",
+			 "help");
+	parser.addOption(OptList, OptionNone, "List all cameras", "list");
 
 	options = parser.parse(argc, argv);
 	if (!options.valid())
diff --git a/src/cam/options.cpp b/src/cam/options.cpp
index c13022ce1b84..204081f3cd8e 100644
--- a/src/cam/options.cpp
+++ b/src/cam/options.cpp
@@ -12,6 +12,30 @@ 
 
 #include "options.h"
 
+/* -----------------------------------------------------------------------------
+ * Option
+ */
+
+const char *Option::typeName() const
+{
+	switch (type) {
+	case OptionNone:
+		return "none";
+
+	case OptionInteger:
+		return "integer";
+
+	case OptionString:
+		return "string";
+	}
+
+	return "unknown";
+}
+
+/* -----------------------------------------------------------------------------
+ * OptionBase<T>
+ */
+
 template <typename T>
 bool OptionsBase<T>::valid() const
 {
@@ -25,11 +49,45 @@  bool OptionsBase<T>::isSet(const T &opt) const
 }
 
 template <typename T>
-const std::string &OptionsBase<T>::operator[](const T &opt) const
+const OptionValue &OptionsBase<T>::operator[](const T &opt) const
 {
 	return values_.find(opt)->second;
 }
 
+template <typename T>
+bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
+				const char *optarg)
+{
+	OptionValue value;
+
+	switch (option.type) {
+	case OptionNone:
+		break;
+
+	case OptionInteger:
+		unsigned int integer;
+
+		if (optarg) {
+			char *endptr;
+			integer = strtoul(optarg, &endptr, 10);
+			if (*endptr != '\0')
+				return false;
+		} else {
+			integer = 0;
+		}
+
+		value = OptionValue(integer);
+		break;
+
+	case OptionString:
+		value = OptionValue(optarg ? optarg : "");
+		break;
+	}
+
+	values_[opt] = value;
+	return true;
+}
+
 template <typename T>
 void OptionsBase<T>::clear()
 {
@@ -38,8 +96,53 @@  void OptionsBase<T>::clear()
 
 template class OptionsBase<int>;
 
-bool OptionsParser::addOption(int opt, const char *help, const char *name,
-			      OptionArgument argument, const char *argumentName)
+/* -----------------------------------------------------------------------------
+ * OptionValue
+ */
+
+OptionValue::OptionValue()
+	: type_(OptionNone)
+{
+}
+
+OptionValue::OptionValue(int value)
+	: type_(OptionInteger), integer_(value)
+{
+}
+
+OptionValue::OptionValue(const char *value)
+	: type_(OptionString), string_(value)
+{
+}
+
+OptionValue::OptionValue(const std::string &value)
+	: type_(OptionString), string_(value)
+{
+}
+
+OptionValue::operator int() const
+{
+	if (type_ != OptionInteger)
+		return 0;
+
+	return integer_;
+}
+
+OptionValue::operator std::string() const
+{
+	if (type_ != OptionString)
+		return std::string();
+
+	return string_;
+}
+
+/* -----------------------------------------------------------------------------
+ * OptionsParser
+ */
+
+bool OptionsParser::addOption(int opt, OptionType type, const char *help,
+			      const char *name, OptionArgument argument,
+			      const char *argumentName)
 {
 	/*
 	 * Options must have at least a short or long name, and a text message.
@@ -56,7 +159,8 @@  bool OptionsParser::addOption(int opt, const char *help, const char *name,
 	if (optionsMap_.find(opt) != optionsMap_.end())
 		return false;
 
-	options_.push_back(Option({ opt, name, argument, argumentName, help }));
+	options_.push_back(Option({ opt, type, name, argument, argumentName,
+				    help }));
 	optionsMap_[opt] = &options_.back();
 	return true;
 }
@@ -126,7 +230,13 @@  OptionsParser::Options OptionsParser::parse(int argc, char **argv)
 			break;
 		}
 
-		options.values_[c] = optarg ? optarg : "";
+		const Option &option = *optionsMap_[c];
+		if (!options.parseValue(c, option, optarg)) {
+			parseValueError(option);
+			usage();
+			options.clear();
+			break;
+		}
 	}
 
 	return options;
@@ -193,3 +303,16 @@  void OptionsParser::usage()
 		}
 	}
 }
+
+void OptionsParser::parseValueError(const Option &option)
+{
+	std::string optionName;
+
+	if (option.name)
+		optionName = "--" + std::string(option.name);
+	else
+		optionName = "-" + static_cast<char>(option.opt);
+
+	std::cerr << "Can't parse " << option.typeName()
+		  << " argument for option " << optionName << std::endl;
+}
diff --git a/src/cam/options.h b/src/cam/options.h
index b9b7bd258c03..8b611d374fd5 100644
--- a/src/cam/options.h
+++ b/src/cam/options.h
@@ -17,8 +17,15 @@  enum OptionArgument {
 	ArgumentOptional,
 };
 
+enum OptionType {
+	OptionNone,
+	OptionInteger,
+	OptionString,
+};
+
 struct Option {
 	int opt;
+	OptionType type;
 	const char *name;
 	OptionArgument argument;
 	const char *argumentName;
@@ -26,20 +33,45 @@  struct Option {
 
 	bool hasShortOption() const { return isalnum(opt); }
 	bool hasLongOption() const { return name != nullptr; }
+	const char *typeName() const;
 };
 
+class OptionValue;
+
 template <typename T>
 class OptionsBase
 {
 public:
 	bool valid() const;
 	bool isSet(const T &opt) const;
-	const std::string &operator[](const T &opt) const;
+	const OptionValue &operator[](const T &opt) const;
 
 private:
 	friend class OptionsParser;
-	std::map<T, std::string> values_;
+
+	bool parseValue(const T &opt, const Option &option, const char *value);
 	void clear();
+
+	std::map<T, OptionValue> values_;
+};
+
+class OptionValue
+{
+public:
+	OptionValue();
+	OptionValue(int value);
+	OptionValue(const char *value);
+	OptionValue(const std::string &value);
+
+	OptionType type() const { return type_; }
+
+	operator int() const;
+	operator std::string() const;
+
+private:
+	OptionType type_;
+	int integer_;
+	std::string string_;
 };
 
 class OptionsParser
@@ -49,7 +81,8 @@  public:
 	{
 	};
 
-	bool addOption(int opt, const char *help, const char *name = nullptr,
+	bool addOption(int opt, OptionType type, const char *help,
+		       const char *name = nullptr,
 		       OptionArgument argument = ArgumentNone,
 		       const char *argumentName = nullptr);
 
@@ -57,6 +90,8 @@  public:
 	void usage();
 
 private:
+	void parseValueError(const Option &option);
+
 	std::list<Option> options_;
 	std::map<unsigned int, Option *> optionsMap_;
 };