[libcamera-devel] [PATCH] libcamera: controls: Initialize ControlInfoMap::idmap_

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Aug 12, 2021 at 07:39:07PM +0200, Jacopo Mondi wrote:
> The compiler generated constructor does not initialize the
> ControlInfoMap::idmap_ field.
> 
> Fix this by explicitly defining an empty constructor for the class.
> 
> Reported-by: Coverity CID=354657
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/controls.h | 2 +-
>  src/libcamera/controls.cpp   | 8 ++++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 9b0d5a545301..7352c62ec03c 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -307,7 +307,7 @@ class ControlInfoMap : private std::unordered_map<const ControlId *, ControlInfo
>  public:
>  	using Map = std::unordered_map<const ControlId *, ControlInfo>;
>  
> -	ControlInfoMap() = default;
> +	ControlInfoMap();
>  	ControlInfoMap(const ControlInfoMap &other) = default;
>  	ControlInfoMap(std::initializer_list<Map::value_type> init,
>  		       const ControlIdMap &idmap);

There's also the option of declaring

	const ControlIdMap *idmap_ = nullptr;

I'm not sure what the pros and cons are, I'm pointing it out hoping that
someone could tell me :-)

Regardless of which option you pick,

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

> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 5c05ba4a7cd0..83f61fd96b24 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -636,6 +636,14 @@ std::string ControlInfo::toString() const
>   * \brief The base std::unsorted_map<> container
>   */
>  
> +/**
> + * \brief Construct an empty ControlInfoMap
> + */
> +ControlInfoMap::ControlInfoMap()
> +	: idmap_(nullptr)
> +{
> +}
> +
>  /**
>   * \fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)
>   * \brief Copy constructor, construct a ControlInfoMap from a copy of \a other

-- 
Regards,

Laurent Pinchart