[libcamera-devel] [PATCH] libcamera: control: initialise control info to ControlTypeNone by default

[Jacopo Mondi]

Hi Christian

On Fri, Aug 26, 2022 at 07:34:26PM +0200, Christian Rauch via libcamera-devel wrote:
> The default ControlInfo constructor allows to partially initialised the
> min/max/def values. Uninitialised values are assigned to 0 by default. This
> implicit initialisation makes it impossible to distinguish between and
> uninitialised and an explicitly 0-initialised ControlValue.
>
> Default construct the ControlValue in the ControlInfo default contructor to
> explicitly represent uninitialised values by the ControlTypeNone type.
>
> Signed-off-by: Christian Rauch <Rauch.Christian at gmx.de>

I think the idea is good: make sure we don't leave the ControlInfo
members unintentionally initialized to 0. This would certainly lead to
a more robust code base.

However, as soon as we try to access a min/max/def which is now
initialized we get into a runtime error, and I'm not sure in how many
places in the code base we happily access a 0-initialized member
without noticing.

Hence, I think this patch is worth being run through  at least CTS,
preferably throught all the run-time tests we, and the projects using,
libcamera have (I'm thinking libcamera-apps, in example).

We'll sync internally on how to give this patch a good brush.

Thanks
   j

> ---
>  include/libcamera/controls.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index ebc168fc..38d0a3e8 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -268,9 +268,9 @@ private:
>  class ControlInfo
>  {
>  public:
> -	explicit ControlInfo(const ControlValue &min = 0,
> -			     const ControlValue &max = 0,
> -			     const ControlValue &def = 0);
> +	explicit ControlInfo(const ControlValue &min = {},
> +			     const ControlValue &max = {},
> +			     const ControlValue &def = {});
>  	explicit ControlInfo(Span<const ControlValue> values,
>  			     const ControlValue &def = {});
>  	explicit ControlInfo(std::set<bool> values, bool def);
> --
> 2.34.1
>