[libcamera-devel] [PATCH] libcamera: Avoid accessing a non existent control
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Tue Nov 17 23:08:59 CET 2020
Hi Kieran,
On 17/11/2020 14:29, Kieran Bingham wrote:
> 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/
Sure... :-)
>> 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())...
Yes, indeed !
>
> 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.
I agree :-)
> --
> Kieran
>
>
>> if (delays_.find(id) == delays_.end())
>> return false;
>>
>
More information about the libcamera-devel
mailing list