[libcamera-devel] [PATCH 4/4] ipa: raspberrypi: fix use of uninitialized fields

[Laurent Pinchart]

Hi Tomi,

On Wed, Oct 07, 2020 at 03:52:21PM +0300, Tomi Valkeinen wrote:
> On 07/10/2020 15:47, Kieran Bingham wrote:
> > On 07/10/2020 12:07, Tomi Valkeinen wrote:
> >> These fields are not initialized, but are used. Set them to 0.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at iki.fi>
> >> ---
> >>  src/ipa/raspberrypi/controller/rpi/agc.hpp | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> >> index ba7ae09..23374d5 100644
> >> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> >> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> >> @@ -116,9 +116,9 @@ private:
> >>  	std::string exposure_mode_name_;
> >>  	std::string constraint_mode_name_;
> >>  	double ev_;
> >> -	double flicker_period_;
> >> -	double fixed_shutter_;
> >> -	double fixed_analogue_gain_;
> >> +	double flicker_period_ = 0;
> > 
> >  = 0.0; ?
> > 
> > Also - this is setting the initialisation in the header definition,
> > rather than the implementation where we would normally do the
> > initialisation. Any reason for that?
> 
> Any reason not to initialize in the header? I think it's much nicer to
> initialize there when you are setting to a simple literal. It's very
> easy to miss the init in the implementation file, especially if you
> have multiple constructors.

That's the current coding style. I didn't even know this was possible
:-) I'm not opposed to reconsidering this, but it should then be changed
globally.

-- 
Regards,

Laurent Pinchart