[libcamera-devel] [PATCH v2] libcamera: yaml_parser: Return nullopt on error

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Aug 03, 2022 at 01:29:37PM +0200, Jacopo Mondi via libcamera-devel wrote:
> The YamlParser::getList<>() function returns an std::optional<> to allow
> callers to identify cases where parsing the .yaml file failed from cases
> where the parsed list is just empty.
> 
> The current implementation returns an empty vector in case of parsing
> errors, which evaluates to std::nullopt but it less explicit.

{} is not an empty vector, as it's not a vector :-)


The current implementation returns a default-constructed std::optional
in case of errors with

	return {};

The returned value is thus equal to std::nullopt, but the code can be
easily misinterpreted as returning an empty vector by a reader.



Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> Change this by returning std::nullopt in case of errors to make the
> intended behaviour more explicit.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/libcamera/yaml_parser.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index 84cb57d6de83..c96e99e1317c 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -319,7 +319,7 @@ template<typename T,
>  std::optional<std::vector<T>> YamlObject::getList() const
>  {
>  	if (type_ != Type::List)
> -		return {};
> +		return std::nullopt;
>  
>  	std::vector<T> values;
>  	values.reserve(list_.size());
> @@ -327,7 +327,7 @@ std::optional<std::vector<T>> YamlObject::getList() const
>  	for (const YamlObject &entry : asList()) {
>  		const auto value = entry.get<T>();
>  		if (!value)
> -			return {};
> +			return std::nullopt;
>  		values.emplace_back(*value);
>  	}
>  

-- 
Regards,

Laurent Pinchart