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

Message ID 20220805161658.11349-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 1a6c7477fda7c6cfbf3483c529aebf64d7ed6a8e
Headers show
Series
  • [libcamera-devel] libcamera: yaml_parser: Return nullopt on error from YamlObject::get()
Related show

Commit Message

Laurent Pinchart Aug. 5, 2022, 4:16 p.m. UTC 
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@gmx.de>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/yaml_parser.cpp | 48 +++++++++++++++++------------------
 1 file changed, 24 insertions(+), 24 deletions(-)


base-commit: 26c82ce13697e1af5950f4935ecff83c6453f351

Comments

Jacopo Mondi Aug. 8, 2022, 7:38 a.m. UTC | #1 
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@gmx.de>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@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
>
Umang Jain Aug. 10, 2022, 6:36 a.m. UTC | #2 
Hi Laurent

Thank you for the patch.

On 8/5/22 21:46, 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@gmx.de>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

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

Patch
diff mbox series 

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);
 }