[PATCH v1 3/3] libcamera: yaml_parser: Use `YamlObject::find()` in contains/operator[]

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Dec 6 02:16:08 CET 2024


Hi Barnabás,

Thank you for the patch.

On Thu, Dec 05, 2024 at 04:34:24PM +0000, Barnabás Pőcze wrote:
> Use `YamlObject::find()` to implement `YamlObject::{contains,operator[]}()`.
> This way there is a single source of truth for dictionary lookups.
> 
> Furthermore, inline `YamlObject::contains()` as it can trivially
> be expressed as a call to `find()`.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> ---
>  include/libcamera/internal/yaml_parser.h |  6 +++++-
>  src/libcamera/yaml_parser.cpp            | 14 ++------------
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> index 796e3e90..20dab2f3 100644
> --- a/include/libcamera/internal/yaml_parser.h
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -207,7 +207,11 @@ public:
>  
>  	const YamlObject &operator[](std::size_t index) const;
>  
> -	bool contains(std::string_view key) const;
> +	bool contains(std::string_view key) const
> +	{
> +		return find(key);
> +	}
> +
>  	const YamlObject &operator[](std::string_view key) const;
>  	const YamlObject *find(std::string_view key) const;
>  
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index da8cb61f..72483ef3 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -369,10 +369,6 @@ const YamlObject &YamlObject::operator[](std::size_t index) const
>   *
>   * \return True if an element exists, false otherwise
>   */
> -bool YamlObject::contains(std::string_view key) const
> -{
> -	return dictionary_.find(key) != dictionary_.end();
> -}
>  
>  /**
>   * \fn YamlObject::operator[](std::string_view key) const
> @@ -387,14 +383,8 @@ bool YamlObject::contains(std::string_view key) const
>   */
>  const YamlObject &YamlObject::operator[](std::string_view key) const
>  {
> -	if (type_ != Type::Dictionary)
> -		return empty;
> -
> -	auto iter = dictionary_.find(key);
> -	if (iter == dictionary_.end())
> -		return empty;
> -
> -	return *iter->second;
> +	auto *child = find(key);

As commented on 2/3 I'd use an explicit type here.

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

> +	return child ? *child : empty;
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list