[libcamera-devel] [PATCH v2 10/24] libcamera: controls: Catch type mismatch in ControlInfoMap

[Kieran Bingham]

Hi Laurent,

On 18/11/2019 01:01, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Fri, Nov 15, 2019 at 05:21:29PM +0100, Jacopo Mondi wrote:
>> On Fri, Nov 08, 2019 at 10:53:55PM +0200, Laurent Pinchart wrote:
>>> ControlInfoMap requires the ControlId and ControlRange of each entry to
>>> have identical types. Check for this and log an error if a mismatch is
>>> detected.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> ---
>>>  src/libcamera/controls.cpp | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>>> index eae0250a92e3..c488b2e4eb3f 100644
>>> --- a/src/libcamera/controls.cpp
>>> +++ b/src/libcamera/controls.cpp
>>> @@ -571,8 +571,19 @@ ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
>>>  void ControlInfoMap::generateIdmap()
>>>  {
>>>  	idmap_.clear();
>>> -	for (const auto &ctrl : *this)
>>> +
>>> +	for (const auto &ctrl : *this) {
>>> +		if (ctrl.first->type() != ctrl.second.min().type()) {
>>> +			LOG(Controls, Error)
>>> +				<< "Control " << utils::hex(ctrl.first->id())
>>> +				<< " type and range type mismatch";
>>> +			idmap_.clear();
>>> +			clear();
>>> +			return;
>>> +		}
>>> +
>>
>> Should we just skip the control or actually abort the whole id map
>> generation ?
> 
> As this is a serious error, I think we should abort it completely, in
> order to show that something went wrong. If you think we should just
> skip the control, we can discuss that.

Is this serious enough to be Fatal? (to be really noisy?)

(Bearing in mind we still need to look at if Fatal messages are
non-fatal when NDEBUG is defined).

--
Kieran

> 
>>>  		idmap_[ctrl.first->id()] = ctrl.first;
>>> +	}
>>>  }
>>>
>>>  /**
> 

-- 
Regards
--
Kieran