[{"id":18743,"web_url":"https://patchwork.libcamera.org/comment/18743/","msgid":"","date":"2021-08-12T19:01:31","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Initialize\n\tControlInfoMap::idmap_","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Aug 12, 2021 at 07:39:07PM +0200, Jacopo Mondi wrote:\n> The compiler generated constructor does not initialize the\n> ControlInfoMap::idmap_ field.\n> \n> Fix this by explicitly defining an empty constructor for the class.\n> \n> Reported-by: Coverity CID=354657\n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/controls.h | 2 +-\n> src/libcamera/controls.cpp | 8 ++++++++\n> 2 files changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 9b0d5a545301..7352c62ec03c 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -307,7 +307,7 @@ class ControlInfoMap : private std::unordered_map public:\n> \tusing Map = std::unordered_map;\n> \n> -\tControlInfoMap() = default;\n> +\tControlInfoMap();\n> \tControlInfoMap(const ControlInfoMap &other) = default;\n> \tControlInfoMap(std::initializer_list init,\n> \t\t const ControlIdMap &idmap);\n\nThere's also the option of declaring\n\n\tconst ControlIdMap *idmap_ = nullptr;\n\nI'm not sure what the pros and cons are, I'm pointing it out hoping that\nsomeone could tell me :-)\n\nRegardless of which option you pick,\n\nReviewed-by: Laurent Pinchart \n\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 5c05ba4a7cd0..83f61fd96b24 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -636,6 +636,14 @@ std::string ControlInfo::toString() const\n> * \\brief The base std::unsorted_map<> container\n> */\n> \n> +/**\n> + * \\brief Construct an empty ControlInfoMap\n> + */\n> +ControlInfoMap::ControlInfoMap()\n> +\t: idmap_(nullptr)\n> +{\n> +}\n> +\n> /**\n> * \\fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)\n> * \\brief Copy constructor, construct a ControlInfoMap from a copy of \\a other","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 11282BD87D\n\tfor ;\n\tThu, 12 Aug 2021 19:01:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 78DE868864;\n\tThu, 12 Aug 2021 21:01:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 00EC568823\n\tfor ;\n\tThu, 12 Aug 2021 21:01:35 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 73350EE;\n\tThu, 12 Aug 2021 21:01:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QMhOjdfx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628794895;\n\tbh=B/SX3hK95P4fmhPKD5bGqW8kDW142wpzFbCm7Z54w7k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QMhOjdfxBUtKMt/kwLJOSvkAkkjeI8EucIPSQF/H3ObTtWbXUJU8h+0WmU27bTF0q\n\tjBppDK9bSJslGCa8AhP19IGCD91z+IU0tgCV+Sgc0Bu33VW3w4pIkXoWJoIQ9v6i1N\n\tFwAvnty6Xo+PTWHW3RpfKmYb2W5XFHxC8R4Mnimk=","Date":"Thu, 12 Aug 2021 22:01:31 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210812173907.204700-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210812173907.204700-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Initialize\n\tControlInfoMap::idmap_","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18831,"web_url":"https://patchwork.libcamera.org/comment/18831/","msgid":"<459bd99d-ddc8-bae3-83ff-72c835223051@ideasonboard.com>","date":"2021-08-16T12:13:26","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Initialize\n\tControlInfoMap::idmap_","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 12/08/2021 20:01, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> Thank you for the patch.\n> \n> On Thu, Aug 12, 2021 at 07:39:07PM +0200, Jacopo Mondi wrote:\n>> The compiler generated constructor does not initialize the\n>> ControlInfoMap::idmap_ field.\n>>\n>> Fix this by explicitly defining an empty constructor for the class.\n>>\n>> Reported-by: Coverity CID=354657\n>> Signed-off-by: Jacopo Mondi \n>> ---\n>> include/libcamera/controls.h | 2 +-\n>> src/libcamera/controls.cpp | 8 ++++++++\n>> 2 files changed, 9 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index 9b0d5a545301..7352c62ec03c 100644\n>> --- a/include/libcamera/controls.h\n>> +++ b/include/libcamera/controls.h\n>> @@ -307,7 +307,7 @@ class ControlInfoMap : private std::unordered_map> public:\n>> \tusing Map = std::unordered_map;\n>> \n>> -\tControlInfoMap() = default;\n>> +\tControlInfoMap();\n>> \tControlInfoMap(const ControlInfoMap &other) = default;\n>> \tControlInfoMap(std::initializer_list init,\n>> \t\t const ControlIdMap &idmap);\n> \n> There's also the option of declaring\n> \n> \tconst ControlIdMap *idmap_ = nullptr;\n> \n> I'm not sure what the pros and cons are, I'm pointing it out hoping that\n> someone could tell me :-)\n> \n\nSo far we've always done initialisation in constructor or initialisation\nlists. Mostly because we didn't realise earlier on that we could\ninitialise in the declarations in the classes I think.\n\nI think declaring in the class inline and reducing the inialiser lists\nmight end up being cleaner. I expect there won't be much difference to\nthe compiler.\n\n\n\n> Regardless of which option you pick,\n> \n\nLikewise,\n\nReviewed-by: Kieran Bingham \n\n\n> Reviewed-by: Laurent Pinchart \n> \n>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>> index 5c05ba4a7cd0..83f61fd96b24 100644\n>> --- a/src/libcamera/controls.cpp\n>> +++ b/src/libcamera/controls.cpp\n>> @@ -636,6 +636,14 @@ std::string ControlInfo::toString() const\n>> * \\brief The base std::unsorted_map<> container\n>> */\n>> \n>> +/**\n>> + * \\brief Construct an empty ControlInfoMap\n>> + */\n>> +ControlInfoMap::ControlInfoMap()\n>> +\t: idmap_(nullptr)\n>> +{\n>> +}\n>> +\n>> /**\n>> * \\fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)\n>> * \\brief Copy constructor, construct a ControlInfoMap from a copy of \\a other\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A63FABD87C\n\tfor ;\n\tMon, 16 Aug 2021 12:13:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4038268894;\n\tMon, 16 Aug 2021 14:13:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A3F568891\n\tfor ;\n\tMon, 16 Aug 2021 14:13:30 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A2983E5;\n\tMon, 16 Aug 2021 14:13:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ARXLGz8x\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629116009;\n\tbh=hRY92rLLM4SF3VD8Dq2WCNLLX9j9Q1g7fdliXp5hdtM=;\n\th=To:Cc:References:From:Subject:Date:In-Reply-To:From;\n\tb=ARXLGz8xy5P6FRf2tANQOfq/gnA+t51NiX1+yNoo6QzWl77+bzoKwqNKdeuQt5onz\n\tyv8yw3KJMMCX1cejYjYXhILKLouJc3c1iWz5EMf4R9IPWgA++NPCAiJtacLjljMOUM\n\tFhaTufCQVZSKVfnNLAz2Ady80PvktdNnUzBgzRKI=","To":"Laurent Pinchart ,\n\tJacopo Mondi ","References":"<20210812173907.204700-1-jacopo@jmondi.org>\n\t","From":"Kieran Bingham ","Message-ID":"<459bd99d-ddc8-bae3-83ff-72c835223051@ideasonboard.com>","Date":"Mon, 16 Aug 2021 13:13:26 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Initialize\n\tControlInfoMap::idmap_","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]