| Message ID | 20220816015414.7462-7-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On Tue, Aug 16, 2022 at 04:54:11AM +0300, Laurent Pinchart via libcamera-devel wrote: > 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> > --- > 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 >
Hi Laurent On Tue, Aug 16, 2022 at 04:54:11AM +0300, Laurent Pinchart via libcamera-devel wrote: > 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> > --- > 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) The usage of int32_t types might not scale if we want to add support for parsing 64-bit values. For now I think it's fine Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > +{ > + 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 >
Hi Jacopo, On Thu, Aug 18, 2022 at 03:44:34PM +0200, Jacopo Mondi wrote: > On Tue, Aug 16, 2022 at 04:54:11AM +0300, Laurent Pinchart via libcamera-devel wrote: > > 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> > > --- > > 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) > > The usage of int32_t types might not scale if we want to add support > for parsing 64-bit values. I thought about it and then decided to leave it for later, but I think I can already change this to use a long (and an unsigned long below) as those are the return types of strtol and strtoul. It won't give us 64-bit support on 32-bit platforms, that will need to be handled when adding 64-bit integer support in any case. > For now I think it's fine > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > +{ > > + 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;
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;
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> --- src/libcamera/yaml_parser.cpp | 160 ++++++++++++++-------------------- 1 file changed, 67 insertions(+), 93 deletions(-)