[libcamera-devel] [PATCH 1/2] cam: Separate options valid() and empty()

[Laurent Pinchart]

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>
---
 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