[libcamera-devel] [PATCH v4 6/9] libcamera: yaml_parser: De-duplicate common code in YamlObject::get()

[Laurent Pinchart]

The specializations of the YamlObject::get() function template for
integer types duplicate code that doesn't directly depend on the
template type argument. Move it to separate helper functions to reduce
the object size.

While at it, rephrase the comment about unsigned integer parsing.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 src/libcamera/yaml_parser.cpp | 160 ++++++++++++++--------------------
 1 file changed, 67 insertions(+), 93 deletions(-)

diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
index 85a52c05e682..3d5efdc4419b 100644
--- a/src/libcamera/yaml_parser.cpp
+++ b/src/libcamera/yaml_parser.cpp
@@ -131,23 +131,65 @@ std::optional<bool> YamlObject::get() const
 	return std::nullopt;
 }
 
+namespace {
+
+bool parseSignedInteger(const std::string &str, int32_t min, int32_t max,
+			int32_t *result)
+{
+	if (str == "")
+		return false;
+
+	char *end;
+
+	errno = 0;
+	long value = std::strtol(str.c_str(), &end, 10);
+
+	if ('\0' != *end || errno == ERANGE || value < min || value > max)
+		return false;
+
+	*result = value;
+	return true;
+}
+
+bool parseUnsignedInteger(const std::string &str, uint32_t max, uint32_t *result)
+{
+	if (str == "")
+		return false;
+
+	/*
+	 * strtoul() accepts strings representing a negative number, in which
+	 * case it negates the converted value. We don't want to silently accept
+	 * negative values and return a large positive number, so check for a
+	 * minus sign (after optional whitespace) and return an error.
+	 */
+	std::size_t found = str.find_first_not_of(" \t");
+	if (found != std::string::npos && str[found] == '-')
+		return false;
+
+	char *end;
+
+	errno = 0;
+	unsigned long value = std::strtoul(str.c_str(), &end, 10);
+
+	if ('\0' != *end || errno == ERANGE || value > max)
+		return false;
+
+	*result = value;
+	return true;
+}
+
+} /* namespace */
+
 template<>
 std::optional<int8_t> YamlObject::get() const
 {
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
+	int32_t value;
 
-	char *end;
-
-	errno = 0;
-	long value = std::strtol(value_.c_str(), &end, 10);
-
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<int8_t>::min() ||
-	    value > std::numeric_limits<int8_t>::max())
+	if (!parseSignedInteger(value_, std::numeric_limits<int8_t>::min(),
+				std::numeric_limits<int8_t>::max(), &value))
 		return std::nullopt;
 
 	return value;
@@ -159,28 +201,10 @@ std::optional<uint8_t> YamlObject::get() const
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
+	uint32_t value;
 
-	/*
-	 * libyaml parses all scalar values as strings. When a string has
-	 * leading spaces before a minus sign, for example "  -10", strtoul
-	 * skips leading spaces, accepts the leading minus sign, and the
-	 * calculated digits are negated as if by unary minus. Rule it out in
-	 * case the user gets a large number when the value is negative.
-	 */
-	std::size_t found = value_.find_first_not_of(" \t");
-	if (found != std::string::npos && value_[found] == '-')
-		return std::nullopt;
-
-	char *end;
-
-	errno = 0;
-	unsigned long value = std::strtoul(value_.c_str(), &end, 10);
-
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<uint8_t>::min() ||
-	    value > std::numeric_limits<uint8_t>::max())
+	if (!parseUnsignedInteger(value_, std::numeric_limits<uint8_t>::max(),
+				  &value))
 		return std::nullopt;
 
 	return value;
@@ -192,17 +216,10 @@ std::optional<int16_t> YamlObject::get() const
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
+	int32_t value;
 
-	char *end;
-
-	errno = 0;
-	long value = std::strtol(value_.c_str(), &end, 10);
-
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<int16_t>::min() ||
-	    value > std::numeric_limits<int16_t>::max())
+	if (!parseSignedInteger(value_, std::numeric_limits<int16_t>::min(),
+				std::numeric_limits<int16_t>::max(), &value))
 		return std::nullopt;
 
 	return value;
@@ -214,28 +231,10 @@ std::optional<uint16_t> YamlObject::get() const
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
+	uint32_t value;
 
-	/*
-	 * libyaml parses all scalar values as strings. When a string has
-	 * leading spaces before a minus sign, for example "  -10", strtoul
-	 * skips leading spaces, accepts the leading minus sign, and the
-	 * calculated digits are negated as if by unary minus. Rule it out in
-	 * case the user gets a large number when the value is negative.
-	 */
-	std::size_t found = value_.find_first_not_of(" \t");
-	if (found != std::string::npos && value_[found] == '-')
-		return std::nullopt;
-
-	char *end;
-
-	errno = 0;
-	unsigned long value = std::strtoul(value_.c_str(), &end, 10);
-
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<uint16_t>::min() ||
-	    value > std::numeric_limits<uint16_t>::max())
+	if (!parseUnsignedInteger(value_, std::numeric_limits<uint16_t>::max(),
+				  &value))
 		return std::nullopt;
 
 	return value;
@@ -247,17 +246,10 @@ std::optional<int32_t> YamlObject::get() const
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
+	int32_t value;
 
-	char *end;
-
-	errno = 0;
-	long value = std::strtol(value_.c_str(), &end, 10);
-
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<int32_t>::min() ||
-	    value > std::numeric_limits<int32_t>::max())
+	if (!parseSignedInteger(value_, std::numeric_limits<int32_t>::min(),
+				std::numeric_limits<int32_t>::max(), &value))
 		return std::nullopt;
 
 	return value;
@@ -269,28 +261,10 @@ std::optional<uint32_t> YamlObject::get() const
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
+	uint32_t value;
 
-	/*
-	 * libyaml parses all scalar values as strings. When a string has
-	 * leading spaces before a minus sign, for example "  -10", strtoul
-	 * skips leading spaces, accepts the leading minus sign, and the
-	 * calculated digits are negated as if by unary minus. Rule it out in
-	 * case the user gets a large number when the value is negative.
-	 */
-	std::size_t found = value_.find_first_not_of(" \t");
-	if (found != std::string::npos && value_[found] == '-')
-		return std::nullopt;
-
-	char *end;
-
-	errno = 0;
-	unsigned long value = std::strtoul(value_.c_str(), &end, 10);
-
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<uint32_t>::min() ||
-	    value > std::numeric_limits<uint32_t>::max())
+	if (!parseUnsignedInteger(value_, std::numeric_limits<uint32_t>::max(),
+				  &value))
 		return std::nullopt;
 
 	return value;
-- 
Regards,

Laurent Pinchart