Re: [PATCH 1/6] arm/mpu: Find MPU region by range

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 30/06/2025 13:45, Hari Limaye wrote:
> Hi Michal,
> 
>  
> 
> Thank you very much for the review.
> 
>  
> 
> I just had a small clarification regarding the following comment:
> 
>>> +
> 
>> +    /* Allow index to be NULL */
> 
>> +    index = index ? index : &_index;
> 
>>If index argument is NULL, why bother setting this internal variable _index?
> 
>  
> 
> This assignment is intended to support `index` as an optional output 
> parameter:
> callers can pass NULL if they only care about the return value. This approach
> avoids repeated `if (index)` checks by aliasing to a local dummy variable 
> upfront.
You don't need multiple if ( index ). Just one at the end of a function by
modifying it to use goto e.g.:

rc = <return_code>;
goto out;

 out:
 if ( index )
        *index = i;

> 
> Would you be happy for me to retain this pattern, renaming the dummy variable 
> to
> make it clearer, e.g.:
> 
> ```
> 
> uint8_t dummy_index;
> 
> index = index ? index : &dummy_index;
> 
> ```
> 
> I would also update the documentation to clarify that index is optional.
This does not address my point about having and setting a variable that may not
be used.
> 
>  
> 
> Alternatively, if you’d prefer to disallow NULL for index and require the 
> caller
> to always provide a valid pointer, I’m happy to change it accordingly.
No, that's not what I had in mind. That said it's up to you to decide how you
want the caller to behave. If you think that obtaining index is not necessary in
all the scenarios, then you should retain your current functionality.

~Michal