[libcamera-devel] [PATCH v2 6/8] cam: options: Add option type handling to options parser

Niklas Söderlund niklas.soderlund at ragnatech.se
Fri Feb 1 10:35:44 CET 2019


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 at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at 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
> 

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list