[libcamera-devel] [PATCH 1/3] libcamera: controls: Use a const ControlValidator

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tue, Aug 10, 2021 at 05:11:32PM +0100, Kieran Bingham wrote:
> The ControlValidator passed to a ControlList constructor
> is used, but not modified.
> 
> Make it const.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

This looks fine, so

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

> ---
>  include/libcamera/controls.h | 6 +++---
>  src/libcamera/controls.cpp   | 6 ++++--
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index de733bd868a6..2c38b23ad443 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -354,8 +354,8 @@ private:
>  
>  public:
>  	ControlList();
> -	ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr);
> -	ControlList(const ControlInfoMap &infoMap, ControlValidator *validator = nullptr);
> +	ControlList(const ControlIdMap &idmap, const ControlValidator *validator = nullptr);
> +	ControlList(const ControlInfoMap &infoMap, const ControlValidator *validator = nullptr);
>  
>  	using iterator = ControlListMap::iterator;
>  	using const_iterator = ControlListMap::const_iterator;
> @@ -413,7 +413,7 @@ private:
>  	const ControlValue *find(unsigned int id) const;
>  	ControlValue *find(unsigned int id);
>  
> -	ControlValidator *validator_;
> +	const ControlValidator *validator_;
>  	const ControlIdMap *idmap_;
>  	const ControlInfoMap *infoMap_;
>  
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 64fd5c296226..a5943700ea89 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -836,7 +836,8 @@ ControlList::ControlList()
>   * controls is provided by controls::controls and can be used as the \a idmap
>   * argument.
>   */
> -ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator)
> +ControlList::ControlList(const ControlIdMap &idmap,
> +			 const ControlValidator *validator)
>  	: validator_(validator), idmap_(&idmap), infoMap_(nullptr)
>  {
>  }
> @@ -846,7 +847,8 @@ ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator)
>   * \param[in] infoMap The ControlInfoMap for the control list target object
>   * \param[in] validator The validator (may be null)
>   */
> -ControlList::ControlList(const ControlInfoMap &infoMap, ControlValidator *validator)
> +ControlList::ControlList(const ControlInfoMap &infoMap,
> +			 const ControlValidator *validator)
>  	: validator_(validator), idmap_(&infoMap.idmap()), infoMap_(&infoMap)
>  {
>  }

-- 
Regards,

Laurent Pinchart