[libcamera-devel] [PATCH] libcamera: yaml_parser: Return nullopt on error from YamlObject::get()

[Jacopo Mondi]

Hi Laurent,

On Fri, Aug 05, 2022 at 07:16:58PM +0300, Laurent Pinchart via libcamera-devel wrote:
> The YamlParser::get<>() function returns an std::optional<> to indicate
> when YAML parsing failed.
>
> The current implementation returns a default constructed std::optional
> in case of errors with
>
>         return {};
>
> This has been reported as generating compiler warnings with a gcc 9.3.0
> arm64 cross-compiler:
>
> ../src/libcamera/yaml_parser.cpp:184:11: error: ‘<anonymous>’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   184 |   return {};
>       |           ^
>
> Replace this with an explicit
>
> 	return std::nullopt;
>
> which fixes the warnings and conveys the purpose more explicitly.
>
> Reported-by: Christian Rauch <Rauch.Christian at gmx.de>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> ---
>  src/libcamera/yaml_parser.cpp | 48 +++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index c96e99e1317c..9162e2250ed4 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -121,24 +121,24 @@ template<>
>  std::optional<bool> YamlObject::get() const
>  {
>  	if (type_ != Type::Value)
> -		return {};
> +		return std::nullopt;
>
>  	if (value_ == "true")
>  		return true;
>  	else if (value_ == "false")
>  		return false;
>
> -	return {};
> +	return std::nullopt;
>  }
>
>  template<>
>  std::optional<int16_t> YamlObject::get() const
>  {
>  	if (type_ != Type::Value)
> -		return {};
> +		return std::nullopt;
>
>  	if (value_ == "")
> -		return {};
> +		return std::nullopt;
>
>  	char *end;
>
> @@ -148,7 +148,7 @@ std::optional<int16_t> YamlObject::get() const
>  	if ('\0' != *end || errno == ERANGE ||
>  	    value < std::numeric_limits<int16_t>::min() ||
>  	    value > std::numeric_limits<int16_t>::max())
> -		return {};
> +		return std::nullopt;
>
>  	return value;
>  }
> @@ -157,10 +157,10 @@ template<>
>  std::optional<uint16_t> YamlObject::get() const
>  {
>  	if (type_ != Type::Value)
> -		return {};
> +		return std::nullopt;
>
>  	if (value_ == "")
> -		return {};
> +		return std::nullopt;
>
>  	/*
>  	 * libyaml parses all scalar values as strings. When a string has
> @@ -171,7 +171,7 @@ std::optional<uint16_t> YamlObject::get() const
>  	 */
>  	std::size_t found = value_.find_first_not_of(" \t");
>  	if (found != std::string::npos && value_[found] == '-')
> -		return {};
> +		return std::nullopt;
>
>  	char *end;
>
> @@ -181,7 +181,7 @@ std::optional<uint16_t> YamlObject::get() const
>  	if ('\0' != *end || errno == ERANGE ||
>  	    value < std::numeric_limits<uint16_t>::min() ||
>  	    value > std::numeric_limits<uint16_t>::max())
> -		return {};
> +		return std::nullopt;
>
>  	return value;
>  }
> @@ -190,10 +190,10 @@ template<>
>  std::optional<int32_t> YamlObject::get() const
>  {
>  	if (type_ != Type::Value)
> -		return {};
> +		return std::nullopt;
>
>  	if (value_ == "")
> -		return {};
> +		return std::nullopt;
>
>  	char *end;
>
> @@ -203,7 +203,7 @@ std::optional<int32_t> YamlObject::get() const
>  	if ('\0' != *end || errno == ERANGE ||
>  	    value < std::numeric_limits<int32_t>::min() ||
>  	    value > std::numeric_limits<int32_t>::max())
> -		return {};
> +		return std::nullopt;
>
>  	return value;
>  }
> @@ -212,10 +212,10 @@ template<>
>  std::optional<uint32_t> YamlObject::get() const
>  {
>  	if (type_ != Type::Value)
> -		return {};
> +		return std::nullopt;
>
>  	if (value_ == "")
> -		return {};
> +		return std::nullopt;
>
>  	/*
>  	 * libyaml parses all scalar values as strings. When a string has
> @@ -226,7 +226,7 @@ std::optional<uint32_t> YamlObject::get() const
>  	 */
>  	std::size_t found = value_.find_first_not_of(" \t");
>  	if (found != std::string::npos && value_[found] == '-')
> -		return {};
> +		return std::nullopt;
>
>  	char *end;
>
> @@ -236,7 +236,7 @@ std::optional<uint32_t> YamlObject::get() const
>  	if ('\0' != *end || errno == ERANGE ||
>  	    value < std::numeric_limits<uint32_t>::min() ||
>  	    value > std::numeric_limits<uint32_t>::max())
> -		return {};
> +		return std::nullopt;
>
>  	return value;
>  }
> @@ -245,10 +245,10 @@ template<>
>  std::optional<double> YamlObject::get() const
>  {
>  	if (type_ != Type::Value)
> -		return {};
> +		return std::nullopt;
>
>  	if (value_ == "")
> -		return {};
> +		return std::nullopt;
>
>  	char *end;
>
> @@ -256,7 +256,7 @@ std::optional<double> YamlObject::get() const
>  	double value = std::strtod(value_.c_str(), &end);
>
>  	if ('\0' != *end || errno == ERANGE)
> -		return {};
> +		return std::nullopt;
>
>  	return value;
>  }
> @@ -265,7 +265,7 @@ template<>
>  std::optional<std::string> YamlObject::get() const
>  {
>  	if (type_ != Type::Value)
> -		return {};
> +		return std::nullopt;
>
>  	return value_;
>  }
> @@ -274,18 +274,18 @@ template<>
>  std::optional<Size> YamlObject::get() const
>  {
>  	if (type_ != Type::List)
> -		return {};
> +		return std::nullopt;
>
>  	if (list_.size() != 2)
> -		return {};
> +		return std::nullopt;
>
>  	auto width = list_[0].value->get<uint32_t>();
>  	if (!width)
> -		return {};
> +		return std::nullopt;
>
>  	auto height = list_[1].value->get<uint32_t>();
>  	if (!height)
> -		return {};
> +		return std::nullopt;
>
>  	return Size(*width, *height);
>  }
>
> base-commit: 26c82ce13697e1af5950f4935ecff83c6453f351
> --
> Regards,
>
> Laurent Pinchart
>