[libcamera-devel] [PATCH] libcamera: controls: Initialize ControlInfoMap::idmap_
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Aug 16 14:13:26 CEST 2021
On 12/08/2021 20:01, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Thu, Aug 12, 2021 at 07:39:07PM +0200, Jacopo Mondi wrote:
>> The compiler generated constructor does not initialize the
>> ControlInfoMap::idmap_ field.
>>
>> Fix this by explicitly defining an empty constructor for the class.
>>
>> Reported-by: Coverity CID=354657
>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>> ---
>> include/libcamera/controls.h | 2 +-
>> src/libcamera/controls.cpp | 8 ++++++++
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
>> index 9b0d5a545301..7352c62ec03c 100644
>> --- a/include/libcamera/controls.h
>> +++ b/include/libcamera/controls.h
>> @@ -307,7 +307,7 @@ class ControlInfoMap : private std::unordered_map<const ControlId *, ControlInfo
>> public:
>> using Map = std::unordered_map<const ControlId *, ControlInfo>;
>>
>> - ControlInfoMap() = default;
>> + ControlInfoMap();
>> ControlInfoMap(const ControlInfoMap &other) = default;
>> ControlInfoMap(std::initializer_list<Map::value_type> init,
>> const ControlIdMap &idmap);
>
> There's also the option of declaring
>
> const ControlIdMap *idmap_ = nullptr;
>
> I'm not sure what the pros and cons are, I'm pointing it out hoping that
> someone could tell me :-)
>
So far we've always done initialisation in constructor or initialisation
lists. Mostly because we didn't realise earlier on that we could
initialise in the declarations in the classes I think.
I think declaring in the class inline and reducing the inialiser lists
might end up being cleaner. I expect there won't be much difference to
the compiler.
> Regardless of which option you pick,
>
Likewise,
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> index 5c05ba4a7cd0..83f61fd96b24 100644
>> --- a/src/libcamera/controls.cpp
>> +++ b/src/libcamera/controls.cpp
>> @@ -636,6 +636,14 @@ std::string ControlInfo::toString() const
>> * \brief The base std::unsorted_map<> container
>> */
>>
>> +/**
>> + * \brief Construct an empty ControlInfoMap
>> + */
>> +ControlInfoMap::ControlInfoMap()
>> + : idmap_(nullptr)
>> +{
>> +}
>> +
>> /**
>> * \fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)
>> * \brief Copy constructor, construct a ControlInfoMap from a copy of \a other
>
More information about the libcamera-devel
mailing list