[libcamera-devel] [PATCH 2/4] ipa: raspberrypi: fix missing initialize of status_

[Tomi Valkeinen]

On 07/10/2020 15:48, Kieran Bingham wrote:
> On 07/10/2020 13:35, Kieran Bingham wrote:
>> Hi Tomi,
>>
>> On 07/10/2020 12:07, Tomi Valkeinen wrote:
>>> Many fields in status_ are not initialized, causing use of uninitialized
>>> memory.
>>>
>>> Drop the code that clears some of the individual fields, and instead
>>> just memset the whole thing.
>>>
>>
>> I think David/Naush' input will be important on this one.
>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at iki.fi>
>>> ---
>>>  src/ipa/raspberrypi/controller/rpi/agc.cpp | 8 +-------
>>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>>
>>> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
>>> index df4d364..9e75178 100644
>>> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
>>> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
>>> @@ -148,14 +148,8 @@ Agc::Agc(Controller *controller)
>>>  	  exposure_mode_(nullptr), constraint_mode_(nullptr),
>>>  	  frame_count_(0), lock_count_(0)
>>>  {
>>> +	memset(&status_, 0, sizeof(status_));
>>>  	ev_ = status_.ev = 1.0;
>>> -	flicker_period_ = status_.flicker_period = 0.0;
>>> -	fixed_shutter_ = status_.fixed_shutter = 0;
>>> -	fixed_analogue_gain_ = status_.fixed_analogue_gain = 0.0;
>>> -	// set to zero initially, so we can tell it's not been calculated
>>
>> Is there any benefit to retaining this comment in some adjusted form?
>>
>> Otherwise, I think generally clearing/initialising the whole status
>> sounds good.
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> Ahem, coming back to correct myself - This patch introduces a defect.
> However you fix it in 4/4 ... so ... well ... There's a problem. But
> you've fixed it ;-)

Oh, indeed. I was a bit too hasty =).

 Tomi