[PATCH] ipa: simple: Initialize ccmEnabled to false

[Barnabás Pőcze]

Hi


2025. 04. 01. 12:17 keltezéssel, Milan Zamazal írta:
> Hi Stanislaw,
> 
> thank you for the patch.
> 
> Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com> writes:
> 
>> ccmEnabled variable is not initialized by default, what result of
>> usage of CCM when the algorithm itself is not enabled and configured.
> 
> Interesting, I thought the default boolean value in C++ is false.
> If true or not, it doesn't harm to set it explicitly.

Sadly a default-initialized `bool` like `bool x;` has indeterminate value:
https://en.cppreference.com/w/cpp/language/default_initialization

"""
The effects of default-initialization are:
   * if T is a (possibly cv-qualified) class type, [...]
   * if T is an array type, [...]
   * otherwise, no initialization is performed [...]

[...]

If no initialization is performed for an object, that object retains an
indeterminate value until that value is replaced.
"""

One needs something like `bool x{}, y = {}, z = bool(), w = false;` at least.

> 
>> The bug manifest itself as seldom reproducible corrupted video stream.
>> Fix by initialize the variable in IPAContext class initialization list.
>>
>> Fixes: ac3068655643 ("libcamera: software_isp: Track whether CCM is enabled")
>> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
>> ---
>>   src/ipa/simple/ipa_context.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
>> index 7dc2cd7ae828..afccf557121e 100644
>> --- a/src/ipa/simple/ipa_context.h
>> +++ b/src/ipa/simple/ipa_context.h
>> @@ -82,7 +82,7 @@ struct IPAFrameContext : public FrameContext {
>>   
>>   struct IPAContext {
>>   	IPAContext(unsigned int frameContextSize)
>> -		: frameContexts(frameContextSize)
>> +		: frameContexts(frameContextSize), ccmEnabled(false)
> 
> Or setting it
> 
>    bool ccmEnabled = false;
> 
> in IPAContext?  Up to the maintainers, what they prefer.

I would generally prefer initializing right where the member is declared
as it is clearer than member init lists in my opinion, and is not affected
negatively when additional constructors are introduced.


Regards,
Barnabás Pőcze

> 
>  From my side, in either case:
> 
> Reviewed-by: Milan Zamazal <mzamazal at redhat.com>
> 
>>   	{
>>   	}
>