[libcamera-devel] [PATCH] libcamera: Avoid accessing a non existent control

[Kieran Bingham]

Hi Jean-Michel,

Thanks for highlighting this issue, and providing a fix.


On 17/11/2020 13:24, Jean-Michel Hautbois wrote:
> When pushing a controlone should check if it exists before accessing it,

s/controlone/control, one/

s/it,/it/

> in order to avoid raising abort and crash.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/libcamera/delayed_controls.cpp | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
> index f1c5f890..422deee5 100644
> --- a/src/libcamera/delayed_controls.cpp
> +++ b/src/libcamera/delayed_controls.cpp
> @@ -147,7 +147,11 @@ bool DelayedControls::queue(const ControlList &controls)
>  
>  	/* Update with new controls. */
>  	for (const auto &control : controls) {
> -		const ControlId *id = device_->controls().idmap().at(control.first);
> +		const ControlIdMap &idmap = device_->controls().idmap();
> +		if (idmap.find(control.first) == idmap.end())
> +			return false;
> +
> +		const ControlId *id = idmap.at(control.first);
>  

This does two look ups, (idmap.find(), idmap.at()).

We could combine those to a single lookup by first doing the find, and
keeping the iterator, then using it (if it is not equal to idmap.end())...


With that fixed,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard>

However, given that this patch is against an out of tree patch - I
anticipate the fix should be squashed into the relevant patch before
integration, so I'll leave this up to Niklas.

--
Kieran


>  		if (delays_.find(id) == delays_.end())
>  			return false;
> 

-- 
Regards
--
Kieran