[libcamera-devel] libcamera: controls: guard ControlInfoMap against nullptr idmap_
diff mbox series

Message ID 20230404-guard-idmap-v1-1-b75c2d924caf@baylibre.com
State Accepted
Commit 3dc2605bda52b627f6e009ef4a3c8360d00e358a
Headers show
Series
  • [libcamera-devel] libcamera: controls: guard ControlInfoMap against nullptr idmap_
Related show

Commit Message

Mattijs Korpershoek April 4, 2023, 4:11 p.m. UTC 
It's possible to construct a Camera with an unsafe controlInfo_.
This is the case in the Simple pipeline, where the camera controls are
not populated.

With Simple, if we attempt to set a Control, we end up with a segfault
because the default constructor for ControlInfoMap doesn't
intialized idmap_ which is initialized at class declaration time as

  const ControlIdMap *idmap_ = nullptr;

Add some safeguards in ControlInfoMap to handle this case.

Link: http://codepad.org/CiLLcPNW
Link: https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
I have build-tested this against master and functionally tested on a
v0.0.4 android integration branch.

I've tested that i'm able to do a camera preview even when the
ScalerCrop control is unavailble.

Note that Jacopo already discussed alternative implementation by making
idmap_ an instance instead of a pointer, but that seemed not a good idea
either:

https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
---
 src/libcamera/controls.cpp | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)


---
base-commit: ac7511dc4c594f567ddff27ccc02c30bf6c00bfd
change-id: 20230404-guard-idmap-1d95f2100aca

Best regards,

Comments

Laurent Pinchart April 5, 2023, 2:47 a.m. UTC | #1 
Hi Mattijs,

Thank you for the patch.

On Tue, Apr 04, 2023 at 06:11:16PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> It's possible to construct a Camera with an unsafe controlInfo_.
> This is the case in the Simple pipeline, where the camera controls are
> not populated.
> 
> With Simple, if we attempt to set a Control, we end up with a segfault
> because the default constructor for ControlInfoMap doesn't
> intialized idmap_ which is initialized at class declaration time as
> 
>   const ControlIdMap *idmap_ = nullptr;
> 
> Add some safeguards in ControlInfoMap to handle this case.
> 
> Link: http://codepad.org/CiLLcPNW
> Link: https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
> Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> I have build-tested this against master and functionally tested on a
> v0.0.4 android integration branch.
> 
> I've tested that i'm able to do a camera preview even when the
> ScalerCrop control is unavailble.
> 
> Note that Jacopo already discussed alternative implementation by making
> idmap_ an instance instead of a pointer, but that seemed not a good idea
> either:
> 
> https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
> ---
>  src/libcamera/controls.cpp | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 6dbf9b348709..b808116c01e5 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -677,6 +677,9 @@ ControlInfoMap::ControlInfoMap(Map &&info, const ControlIdMap &idmap)
>  
>  bool ControlInfoMap::validate()
>  {
> +	if (!idmap_)
> +		return false;
> +
>  	for (const auto &ctrl : *this) {
>  		const ControlId *id = ctrl.first;
>  		auto it = idmap_->find(id->id());
> @@ -719,6 +722,8 @@ bool ControlInfoMap::validate()
>   */
>  ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>  {
> +	ASSERT(idmap_);
> +
>  	return at(idmap_->at(id));
>  }
>  
> @@ -729,6 +734,8 @@ ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>   */
>  const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>  {
> +	ASSERT(idmap_);
> +
>  	return at(idmap_->at(id));
>  }
>  
> @@ -739,6 +746,9 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>   */
>  ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>  {
> +	if (!idmap_)
> +		return 0;
> +
>  	/*
>  	 * The ControlInfoMap and its idmap have a 1:1 mapping between their
>  	 * entries, we can thus just count the matching entries in idmap to
> @@ -755,6 +765,9 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>   */
>  ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>  {
> +	if (!idmap_)
> +		return end();
> +
>  	auto iter = idmap_->find(id);
>  	if (iter == idmap_->end())
>  		return end();
> @@ -770,6 +783,9 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>   */
>  ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
>  {
> +	if (!idmap_)
> +		return end();
> +
>  	auto iter = idmap_->find(id);
>  	if (iter == idmap_->end())
>  		return end();
> 
> ---
> base-commit: ac7511dc4c594f567ddff27ccc02c30bf6c00bfd
> change-id: 20230404-guard-idmap-1d95f2100aca
Laurent Pinchart April 5, 2023, 2:49 a.m. UTC | #2 
On Wed, Apr 05, 2023 at 05:47:41AM +0300, Laurent Pinchart wrote:
> Hi Mattijs,
> 
> Thank you for the patch.
> 
> On Tue, Apr 04, 2023 at 06:11:16PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> > It's possible to construct a Camera with an unsafe controlInfo_.
> > This is the case in the Simple pipeline, where the camera controls are
> > not populated.
> > 
> > With Simple, if we attempt to set a Control, we end up with a segfault
> > because the default constructor for ControlInfoMap doesn't
> > intialized idmap_ which is initialized at class declaration time as
> > 
> >   const ControlIdMap *idmap_ = nullptr;
> > 
> > Add some safeguards in ControlInfoMap to handle this case.
> > 
> > Link: http://codepad.org/CiLLcPNW
> > Link: https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
> > Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Actually, maybe I ask you to extend the ControlInfoMap unit test to
catch the issue ? The test should crash without this patch, and pass
with it. This can be done in a separate patch.

> > ---
> > I have build-tested this against master and functionally tested on a
> > v0.0.4 android integration branch.
> > 
> > I've tested that i'm able to do a camera preview even when the
> > ScalerCrop control is unavailble.
> > 
> > Note that Jacopo already discussed alternative implementation by making
> > idmap_ an instance instead of a pointer, but that seemed not a good idea
> > either:
> > 
> > https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
> > ---
> >  src/libcamera/controls.cpp | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 6dbf9b348709..b808116c01e5 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -677,6 +677,9 @@ ControlInfoMap::ControlInfoMap(Map &&info, const ControlIdMap &idmap)
> >  
> >  bool ControlInfoMap::validate()
> >  {
> > +	if (!idmap_)
> > +		return false;
> > +
> >  	for (const auto &ctrl : *this) {
> >  		const ControlId *id = ctrl.first;
> >  		auto it = idmap_->find(id->id());
> > @@ -719,6 +722,8 @@ bool ControlInfoMap::validate()
> >   */
> >  ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
> >  {
> > +	ASSERT(idmap_);
> > +
> >  	return at(idmap_->at(id));
> >  }
> >  
> > @@ -729,6 +734,8 @@ ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
> >   */
> >  const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
> >  {
> > +	ASSERT(idmap_);
> > +
> >  	return at(idmap_->at(id));
> >  }
> >  
> > @@ -739,6 +746,9 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
> >   */
> >  ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
> >  {
> > +	if (!idmap_)
> > +		return 0;
> > +
> >  	/*
> >  	 * The ControlInfoMap and its idmap have a 1:1 mapping between their
> >  	 * entries, we can thus just count the matching entries in idmap to
> > @@ -755,6 +765,9 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
> >   */
> >  ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
> >  {
> > +	if (!idmap_)
> > +		return end();
> > +
> >  	auto iter = idmap_->find(id);
> >  	if (iter == idmap_->end())
> >  		return end();
> > @@ -770,6 +783,9 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
> >   */
> >  ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
> >  {
> > +	if (!idmap_)
> > +		return end();
> > +
> >  	auto iter = idmap_->find(id);
> >  	if (iter == idmap_->end())
> >  		return end();
> > 
> > ---
> > base-commit: ac7511dc4c594f567ddff27ccc02c30bf6c00bfd
> > change-id: 20230404-guard-idmap-1d95f2100aca
Mattijs Korpershoek April 5, 2023, 6:50 a.m. UTC | #3 
Hi Laurent,

Thank you for your review.

On mer., avril 05, 2023 at 05:49, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> On Wed, Apr 05, 2023 at 05:47:41AM +0300, Laurent Pinchart wrote:
>> Hi Mattijs,
>> 
>> Thank you for the patch.
>> 
>> On Tue, Apr 04, 2023 at 06:11:16PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
>> > It's possible to construct a Camera with an unsafe controlInfo_.
>> > This is the case in the Simple pipeline, where the camera controls are
>> > not populated.
>> > 
>> > With Simple, if we attempt to set a Control, we end up with a segfault
>> > because the default constructor for ControlInfoMap doesn't
>> > intialized idmap_ which is initialized at class declaration time as
>> > 
>> >   const ControlIdMap *idmap_ = nullptr;
>> > 
>> > Add some safeguards in ControlInfoMap to handle this case.
>> > 
>> > Link: http://codepad.org/CiLLcPNW
>> > Link: https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
>> > Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> > Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> 
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Actually, maybe I ask you to extend the ControlInfoMap unit test to
> catch the issue ? The test should crash without this patch, and pass
> with it. This can be done in a separate patch.

Yes, no problem, I will send a V2 extending the ControlInfoMap unit test
in a separate patch.

>
>> > ---
>> > I have build-tested this against master and functionally tested on a
>> > v0.0.4 android integration branch.
>> > 
>> > I've tested that i'm able to do a camera preview even when the
>> > ScalerCrop control is unavailble.
>> > 
>> > Note that Jacopo already discussed alternative implementation by making
>> > idmap_ an instance instead of a pointer, but that seemed not a good idea
>> > either:
>> > 
>> > https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
>> > ---
>> >  src/libcamera/controls.cpp | 16 ++++++++++++++++
>> >  1 file changed, 16 insertions(+)
>> > 
>> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> > index 6dbf9b348709..b808116c01e5 100644
>> > --- a/src/libcamera/controls.cpp
>> > +++ b/src/libcamera/controls.cpp
>> > @@ -677,6 +677,9 @@ ControlInfoMap::ControlInfoMap(Map &&info, const ControlIdMap &idmap)
>> >  
>> >  bool ControlInfoMap::validate()
>> >  {
>> > +	if (!idmap_)
>> > +		return false;
>> > +
>> >  	for (const auto &ctrl : *this) {
>> >  		const ControlId *id = ctrl.first;
>> >  		auto it = idmap_->find(id->id());
>> > @@ -719,6 +722,8 @@ bool ControlInfoMap::validate()
>> >   */
>> >  ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>> >  {
>> > +	ASSERT(idmap_);
>> > +
>> >  	return at(idmap_->at(id));
>> >  }
>> >  
>> > @@ -729,6 +734,8 @@ ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>> >   */
>> >  const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>> >  {
>> > +	ASSERT(idmap_);
>> > +
>> >  	return at(idmap_->at(id));
>> >  }
>> >  
>> > @@ -739,6 +746,9 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>> >   */
>> >  ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>> >  {
>> > +	if (!idmap_)
>> > +		return 0;
>> > +
>> >  	/*
>> >  	 * The ControlInfoMap and its idmap have a 1:1 mapping between their
>> >  	 * entries, we can thus just count the matching entries in idmap to
>> > @@ -755,6 +765,9 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>> >   */
>> >  ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>> >  {
>> > +	if (!idmap_)
>> > +		return end();
>> > +
>> >  	auto iter = idmap_->find(id);
>> >  	if (iter == idmap_->end())
>> >  		return end();
>> > @@ -770,6 +783,9 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>> >   */
>> >  ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
>> >  {
>> > +	if (!idmap_)
>> > +		return end();
>> > +
>> >  	auto iter = idmap_->find(id);
>> >  	if (iter == idmap_->end())
>> >  		return end();
>> > 
>> > ---
>> > base-commit: ac7511dc4c594f567ddff27ccc02c30bf6c00bfd
>> > change-id: 20230404-guard-idmap-1d95f2100aca
>
> -- 
> Regards,
>
> Laurent Pinchart
Jacopo Mondi April 5, 2023, 8:01 a.m. UTC | #4 
Hello Mattijs

On Tue, Apr 04, 2023 at 06:11:16PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> It's possible to construct a Camera with an unsafe controlInfo_.
> This is the case in the Simple pipeline, where the camera controls are
> not populated.
>
> With Simple, if we attempt to set a Control, we end up with a segfault
> because the default constructor for ControlInfoMap doesn't
> intialized idmap_ which is initialized at class declaration time as
>
>   const ControlIdMap *idmap_ = nullptr;
>
> Add some safeguards in ControlInfoMap to handle this case.
>
> Link: http://codepad.org/CiLLcPNW

This won't stay here forever I presume, I'm not sure it should be in
the git history. A minor detail, can be removed when applying

> Link: https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
> Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
   j

> ---
> I have build-tested this against master and functionally tested on a
> v0.0.4 android integration branch.
>
> I've tested that i'm able to do a camera preview even when the
> ScalerCrop control is unavailble.
>
> Note that Jacopo already discussed alternative implementation by making
> idmap_ an instance instead of a pointer, but that seemed not a good idea
> either:
>
> https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
> ---
>  src/libcamera/controls.cpp | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 6dbf9b348709..b808116c01e5 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -677,6 +677,9 @@ ControlInfoMap::ControlInfoMap(Map &&info, const ControlIdMap &idmap)
>
>  bool ControlInfoMap::validate()
>  {
> +	if (!idmap_)
> +		return false;
> +
>  	for (const auto &ctrl : *this) {
>  		const ControlId *id = ctrl.first;
>  		auto it = idmap_->find(id->id());
> @@ -719,6 +722,8 @@ bool ControlInfoMap::validate()
>   */
>  ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>  {
> +	ASSERT(idmap_);
> +
>  	return at(idmap_->at(id));
>  }
>
> @@ -729,6 +734,8 @@ ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>   */
>  const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>  {
> +	ASSERT(idmap_);
> +
>  	return at(idmap_->at(id));
>  }
>
> @@ -739,6 +746,9 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>   */
>  ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>  {
> +	if (!idmap_)
> +		return 0;
> +
>  	/*
>  	 * The ControlInfoMap and its idmap have a 1:1 mapping between their
>  	 * entries, we can thus just count the matching entries in idmap to
> @@ -755,6 +765,9 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>   */
>  ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>  {
> +	if (!idmap_)
> +		return end();
> +
>  	auto iter = idmap_->find(id);
>  	if (iter == idmap_->end())
>  		return end();
> @@ -770,6 +783,9 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>   */
>  ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
>  {
> +	if (!idmap_)
> +		return end();
> +
>  	auto iter = idmap_->find(id);
>  	if (iter == idmap_->end())
>  		return end();
>
> ---
> base-commit: ac7511dc4c594f567ddff27ccc02c30bf6c00bfd
> change-id: 20230404-guard-idmap-1d95f2100aca
>
> Best regards,
> --
> Mattijs Korpershoek <mkorpershoek@baylibre.com>
>
Mattijs Korpershoek April 5, 2023, 8:06 a.m. UTC | #5 
Hi Jacopo,

Thank you for your review.

On mer., avril 05, 2023 at 10:01, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:

> Hello Mattijs
>
> On Tue, Apr 04, 2023 at 06:11:16PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
>> It's possible to construct a Camera with an unsafe controlInfo_.
>> This is the case in the Simple pipeline, where the camera controls are
>> not populated.
>>
>> With Simple, if we attempt to set a Control, we end up with a segfault
>> because the default constructor for ControlInfoMap doesn't
>> intialized idmap_ which is initialized at class declaration time as
>>
>>   const ControlIdMap *idmap_ = nullptr;
>>
>> Add some safeguards in ControlInfoMap to handle this case.
>>
>> Link: http://codepad.org/CiLLcPNW
>
> This won't stay here forever I presume, I'm not sure it should be in
> the git history. A minor detail, can be removed when applying

Since I'm spinning up a v2 with an added unit test, I will move this
codepad link to the cover letter instead.

>
>> Link: https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
>> Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>    j
>
>> ---
>> I have build-tested this against master and functionally tested on a
>> v0.0.4 android integration branch.
>>
>> I've tested that i'm able to do a camera preview even when the
>> ScalerCrop control is unavailble.
>>
>> Note that Jacopo already discussed alternative implementation by making
>> idmap_ an instance instead of a pointer, but that seemed not a good idea
>> either:
>>
>> https://lists.libcamera.org/pipermail/libcamera-devel/2023-April/037439.html
>> ---
>>  src/libcamera/controls.cpp | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> index 6dbf9b348709..b808116c01e5 100644
>> --- a/src/libcamera/controls.cpp
>> +++ b/src/libcamera/controls.cpp
>> @@ -677,6 +677,9 @@ ControlInfoMap::ControlInfoMap(Map &&info, const ControlIdMap &idmap)
>>
>>  bool ControlInfoMap::validate()
>>  {
>> +	if (!idmap_)
>> +		return false;
>> +
>>  	for (const auto &ctrl : *this) {
>>  		const ControlId *id = ctrl.first;
>>  		auto it = idmap_->find(id->id());
>> @@ -719,6 +722,8 @@ bool ControlInfoMap::validate()
>>   */
>>  ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>>  {
>> +	ASSERT(idmap_);
>> +
>>  	return at(idmap_->at(id));
>>  }
>>
>> @@ -729,6 +734,8 @@ ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
>>   */
>>  const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>>  {
>> +	ASSERT(idmap_);
>> +
>>  	return at(idmap_->at(id));
>>  }
>>
>> @@ -739,6 +746,9 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
>>   */
>>  ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>>  {
>> +	if (!idmap_)
>> +		return 0;
>> +
>>  	/*
>>  	 * The ControlInfoMap and its idmap have a 1:1 mapping between their
>>  	 * entries, we can thus just count the matching entries in idmap to
>> @@ -755,6 +765,9 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
>>   */
>>  ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>>  {
>> +	if (!idmap_)
>> +		return end();
>> +
>>  	auto iter = idmap_->find(id);
>>  	if (iter == idmap_->end())
>>  		return end();
>> @@ -770,6 +783,9 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
>>   */
>>  ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
>>  {
>> +	if (!idmap_)
>> +		return end();
>> +
>>  	auto iter = idmap_->find(id);
>>  	if (iter == idmap_->end())
>>  		return end();
>>
>> ---
>> base-commit: ac7511dc4c594f567ddff27ccc02c30bf6c00bfd
>> change-id: 20230404-guard-idmap-1d95f2100aca
>>
>> Best regards,
>> --
>> Mattijs Korpershoek <mkorpershoek@baylibre.com>
>>

Patch
diff mbox series 

diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 6dbf9b348709..b808116c01e5 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -677,6 +677,9 @@  ControlInfoMap::ControlInfoMap(Map &&info, const ControlIdMap &idmap)
 
 bool ControlInfoMap::validate()
 {
+	if (!idmap_)
+		return false;
+
 	for (const auto &ctrl : *this) {
 		const ControlId *id = ctrl.first;
 		auto it = idmap_->find(id->id());
@@ -719,6 +722,8 @@  bool ControlInfoMap::validate()
  */
 ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
 {
+	ASSERT(idmap_);
+
 	return at(idmap_->at(id));
 }
 
@@ -729,6 +734,8 @@  ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
  */
 const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
 {
+	ASSERT(idmap_);
+
 	return at(idmap_->at(id));
 }
 
@@ -739,6 +746,9 @@  const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
  */
 ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
 {
+	if (!idmap_)
+		return 0;
+
 	/*
 	 * The ControlInfoMap and its idmap have a 1:1 mapping between their
 	 * entries, we can thus just count the matching entries in idmap to
@@ -755,6 +765,9 @@  ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
  */
 ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
 {
+	if (!idmap_)
+		return end();
+
 	auto iter = idmap_->find(id);
 	if (iter == idmap_->end())
 		return end();
@@ -770,6 +783,9 @@  ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
  */
 ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
 {
+	if (!idmap_)
+		return end();
+
 	auto iter = idmap_->find(id);
 	if (iter == idmap_->end())
 		return end();