[libcamera-devel] [PATCH 1/2] cam: Separate options valid() and empty()
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Mar 27 01:39:06 CET 2019
Hi Laurent,
Thanks for your work.
On 2019-03-23 09:31:24 +0200, Laurent Pinchart wrote:
> An empty option list is not necessarily an error. Add a new empty()
> function to test the option list for emptiness, and modify the valid()
> function to only notify parsing errors. As a side effect this allows
> accessing partially parsed options, which may be useful in the future.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Nice idea,
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/cam/main.cpp | 7 +++++--
> src/cam/options.cpp | 34 +++++++++++++++-------------------
> src/cam/options.h | 5 ++++-
> 3 files changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 1ca7862bf237..e7490c32f99a 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -67,9 +67,12 @@ static int parseOptions(int argc, char *argv[])
> parser.addOption(OptList, OptionNone, "List all cameras", "list");
>
> options = parser.parse(argc, argv);
> - if (!options.valid() || options.isSet(OptHelp)) {
> + if (!options.valid())
> + return -EINVAL;
> +
> + if (options.empty() || options.isSet(OptHelp)) {
> parser.usage();
> - return !options.valid() ? -EINVAL : -EINTR;
> + return options.empty() ? -EINVAL : -EINTR;
> }
>
> return 0;
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index 655aa36bb9c9..f053a31d6ea1 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -39,10 +39,16 @@ const char *Option::typeName() const
> * OptionBase<T>
> */
>
> +template<typename T>
> +bool OptionsBase<T>::empty() const
> +{
> + return values_.empty();
> +}
> +
> template<typename T>
> bool OptionsBase<T>::valid() const
> {
> - return !values_.empty();
> + return valid_;
> }
>
> template<typename T>
> @@ -100,12 +106,6 @@ bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
> return true;
> }
>
> -template<typename T>
> -void OptionsBase<T>::clear()
> -{
> - values_.clear();
> -}
> -
> template class OptionsBase<int>;
> template class OptionsBase<std::string>;
>
> @@ -165,21 +165,18 @@ KeyValueParser::Options KeyValueParser::parse(const char *arguments)
>
> if (optionsMap_.find(key) == optionsMap_.end()) {
> std::cerr << "Invalid option " << key << std::endl;
> - options.clear();
> - break;
> + return options;
> }
>
> OptionArgument arg = optionsMap_[key].argument;
> if (value.empty() && arg == ArgumentRequired) {
> std::cerr << "Option " << key << " requires an argument"
> << std::endl;
> - options.clear();
> - break;
> + return options;
> } else if (!value.empty() && arg == ArgumentNone) {
> std::cerr << "Option " << key << " takes no argument"
> << std::endl;
> - options.clear();
> - break;
> + return options;
> }
>
> const Option &option = optionsMap_[key];
> @@ -187,11 +184,11 @@ KeyValueParser::Options KeyValueParser::parse(const char *arguments)
> std::cerr << "Failed to parse '" << value << "' as "
> << option.typeName() << " for option " << key
> << std::endl;
> - options.clear();
> - break;
> + return options;
> }
> }
>
> + options.valid_ = true;
> return options;
> }
>
> @@ -412,19 +409,18 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
> std::cerr << argv[optind - 1] << std::endl;
>
> usage();
> - options.clear();
> - break;
> + return options;
> }
>
> const Option &option = *optionsMap_[c];
> if (!options.parseValue(c, option, optarg)) {
> parseValueError(option);
> usage();
> - options.clear();
> - break;
> + return options;
> }
> }
>
> + options.valid_ = true;
> return options;
> }
>
> diff --git a/src/cam/options.h b/src/cam/options.h
> index 745f4a4a3a43..0b0444c2db42 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -45,6 +45,9 @@ template<typename T>
> class OptionsBase
> {
> public:
> + OptionsBase() : valid_(false) {}
> +
> + bool empty() const;
> bool valid() const;
> bool isSet(const T &opt) const;
> const OptionValue &operator[](const T &opt) const;
> @@ -54,9 +57,9 @@ private:
> friend class OptionsParser;
>
> bool parseValue(const T &opt, const Option &option, const char *value);
> - void clear();
>
> std::map<T, OptionValue> values_;
> + bool valid_;
> };
>
> class KeyValueParser
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list