From acf721323086cae254bba63e9953a7730162e8aa Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Tue, 16 Aug 2022 00:19:21 +0300
Subject: libcamera: yaml_parser: De-duplicate common code in YamlObject::get()

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@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/yaml_parser.cpp | 157 ++++++++++++++++++------------------------
 1 file changed, 66 insertions(+), 91 deletions(-)

(limited to 'src')

diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
index 85a52c05..84cffd0d 100644
--- a/src/libcamera/yaml_parser.cpp
+++ b/src/libcamera/yaml_parser.cpp
@@ -131,111 +131,111 @@ std::optional<bool> YamlObject::get() const
 	return std::nullopt;
 }
 
-template<>
-std::optional<int8_t> YamlObject::get() const
-{
-	if (type_ != Type::Value)
-		return std::nullopt;
+namespace {
 
-	if (value_ == "")
-		return std::nullopt;
+bool parseSignedInteger(const std::string &str, long min, long max,
+			long *result)
+{
+	if (str == "")
+		return false;
 
 	char *end;
 
 	errno = 0;
-	long value = std::strtol(value_.c_str(), &end, 10);
+	long value = std::strtol(str.c_str(), &end, 10);
 
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<int8_t>::min() ||
-	    value > std::numeric_limits<int8_t>::max())
-		return std::nullopt;
+	if ('\0' != *end || errno == ERANGE || value < min || value > max)
+		return false;
 
-	return value;
+	*result = value;
+	return true;
 }
 
-template<>
-std::optional<uint8_t> YamlObject::get() const
+bool parseUnsignedInteger(const std::string &str, unsigned long max,
+			  unsigned long *result)
 {
-	if (type_ != Type::Value)
-		return std::nullopt;
-
-	if (value_ == "")
-		return std::nullopt;
+	if (str == "")
+		return false;
 
 	/*
-	 * 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.
+	 * 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 = value_.find_first_not_of(" \t");
-	if (found != std::string::npos && value_[found] == '-')
-		return std::nullopt;
+	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(value_.c_str(), &end, 10);
+	unsigned long value = std::strtoul(str.c_str(), &end, 10);
 
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<uint8_t>::min() ||
-	    value > std::numeric_limits<uint8_t>::max())
-		return std::nullopt;
+	if ('\0' != *end || errno == ERANGE || value > max)
+		return false;
 
-	return value;
+	*result = value;
+	return true;
 }
 
+} /* namespace */
+
 template<>
-std::optional<int16_t> YamlObject::get() const
+std::optional<int8_t> YamlObject::get() const
 {
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
+	long value;
+
+	if (!parseSignedInteger(value_, std::numeric_limits<int8_t>::min(),
+				std::numeric_limits<int8_t>::max(), &value))
 		return std::nullopt;
 
-	char *end;
+	return value;
+}
 
-	errno = 0;
-	long value = std::strtol(value_.c_str(), &end, 10);
+template<>
+std::optional<uint8_t> YamlObject::get() const
+{
+	if (type_ != Type::Value)
+		return std::nullopt;
 
-	if ('\0' != *end || errno == ERANGE ||
-	    value < std::numeric_limits<int16_t>::min() ||
-	    value > std::numeric_limits<int16_t>::max())
+	unsigned long value;
+
+	if (!parseUnsignedInteger(value_, std::numeric_limits<uint8_t>::max(),
+				  &value))
 		return std::nullopt;
 
 	return value;
 }
 
 template<>
-std::optional<uint16_t> YamlObject::get() const
+std::optional<int16_t> YamlObject::get() const
 {
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
+	long 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] == '-')
+	if (!parseSignedInteger(value_, std::numeric_limits<int16_t>::min(),
+				std::numeric_limits<int16_t>::max(), &value))
 		return std::nullopt;
 
-	char *end;
+	return value;
+}
 
-	errno = 0;
-	unsigned long value = std::strtoul(value_.c_str(), &end, 10);
+template<>
+std::optional<uint16_t> YamlObject::get() const
+{
+	if (type_ != Type::Value)
+		return std::nullopt;
+
+	unsigned long value;
 
-	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 +247,10 @@ std::optional<int32_t> YamlObject::get() const
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
-
-	char *end;
+	long value;
 
-	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 +262,10 @@ std::optional<uint32_t> YamlObject::get() const
 	if (type_ != Type::Value)
 		return std::nullopt;
 
-	if (value_ == "")
-		return std::nullopt;
-
-	/*
-	 * 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);
+	unsigned long value;
 
-	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;
-- 
cgit v1.2.1