[{"id":3016,"web_url":"https://patchwork.libcamera.org/comment/3016/","msgid":"<20191114081351.GH7353@bigcity.dyn.berto.se>","date":"2019-11-14T08:13:51","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-11-08 22:53:48 +0200, Laurent Pinchart wrote:\n> The ControlList count() and find() methods use at() to lookup the\n> control numerical ID in the idmap_. This causes an exception to be\n> thrown if the ID doesn't exist in the map. Fix it by using the find()\n> method instead.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/controls.cpp | 18 +++++++++++++++---\n> 1 file changed, 15 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 93ad2fc6a276..0c7cd449ad64 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> */\n> ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> {\n> -\treturn count(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn 0;\n> +\n> +\treturn count(iter->second);\n> }\n> \n> /**\n> @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> */\n> ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> {\n> -\treturn find(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn end();\n> +\n> +\treturn find(iter->second);\n> }\n> \n> /**\n> @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> */\n> ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> {\n> -\treturn find(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn end();\n> +\n> +\treturn find(iter->second);\n> }\n> \n> /**\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 71C356136F\n\tfor ;\n\tThu, 14 Nov 2019 09:13:53 +0100 (CET)","by mail-lf1-x141.google.com with SMTP id b20so4288873lfp.4\n\tfor ;\n\tThu, 14 Nov 2019 00:13:53 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\ty6sm1760595ljm.95.2019.11.14.00.13.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 14 Nov 2019 00:13:52 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=4OxNTR6P+4p1T4oVkuTj4F156/aFyZqp1vBq8QulFh4=;\n\tb=YVe77r32RHUMZiXGhH1M7Bq8F/1Bi3AjqmZH//LMNMww1ec6cjdubx1i5z2W/IqWl8\n\tGg+qPAKd8yIjOxcgjqJ/8cptIPCsw/oKTLcungqYgT7BOF5n/peth8CbyCJ8SZud2NDq\n\tOtGkHDbf3hT25aFu3ORtj0XhK50/ZkiG7dfIpLm7pWEgC/5+xC1hawihBrkfD9zF90fs\n\tIiXpwxRKiMIO6D05+c0pXI8crhLdWk2s92xPkzn7ayfcwMLH+NUoBiL/KFMVfF0mAPQs\n\t2g4EDYSpkaniof5TqlXTcD5laC0dUsqSXmQW8w9/Qlw0deGnFVpZCIudqV0MC7JbtFiB\n\tGL0w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=4OxNTR6P+4p1T4oVkuTj4F156/aFyZqp1vBq8QulFh4=;\n\tb=Z4Qb2aqTgKdsnfuH5KayMBsG9eIz9XW4cgKDpaDIFMV6emp8QpQ5415I/2s82c2gMf\n\tsbPndDKKaYh4ePuTxmGA+x8Y83nncszH1pHV2LuDlF//+VrY0NuPCXSH6jK5sgt5d2ha\n\t2yLLm/QDt30tfEAu4vzn7w69rdxqPrHxi7wa7tPMbI+P8rxH6nAV80txA7+DBFh8b9bq\n\tj0s/7HlDaeixuMh1MTdM31MbXO8cDycbt5DYE6TOfdU0Sg4p95dmfY6vZTWup9g4A7wQ\n\tO504kL7lOAOkUAStgpgtCpmpsvh+KBTK/LXDB1BUJYGK9ejA+wGnxnLprFR8CKw/D4R8\n\tM04w==","X-Gm-Message-State":"APjAAAUiShNEkg3t3yU30kWcrYnwUFyPHUSCmSxZ9Arv6I2+LP2Lkz29\n\tL8TQeOq/BtGdO+rd1KovslUP3A==","X-Google-Smtp-Source":"APXvYqzbbpPmEkhG2LmaOLUyUfC7Ttq2lp2VgxK9nA3ut9QyMmDlLVxdooMr3LHpmaIcRfCVhqGQGA==","X-Received":"by 2002:a19:e018:: with SMTP id\n\tx24mr5676017lfg.191.1573719232723; \n\tThu, 14 Nov 2019 00:13:52 -0800 (PST)","Date":"Thu, 14 Nov 2019 09:13:51 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191114081351.GH7353@bigcity.dyn.berto.se>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Thu, 14 Nov 2019 08:13:53 -0000"}},{"id":3023,"web_url":"https://patchwork.libcamera.org/comment/3023/","msgid":"<20191115160045.jyqjzpn5eup52vgy@uno.localdomain>","date":"2019-11-15T16:00:45","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Nov 08, 2019 at 10:53:48PM +0200, Laurent Pinchart wrote:\n> The ControlList count() and find() methods use at() to lookup the\n> control numerical ID in the idmap_. This causes an exception to be\n> thrown if the ID doesn't exist in the map. Fix it by using the find()\n> method instead.\n\nBe aware that ControlList::at() (const) still uses at() and could\npotentially raise an exception as well, while I think we clearly\nprescribes that libcamera does not uses them.\n\nIf that's fine please have mine\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/controls.cpp | 18 +++++++++++++++---\n> 1 file changed, 15 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 93ad2fc6a276..0c7cd449ad64 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> */\n> ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> {\n> -\treturn count(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn 0;\n> +\n> +\treturn count(iter->second);\n> }\n>\n> /**\n> @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> */\n> ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> {\n> -\treturn find(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn end();\n> +\n> +\treturn find(iter->second);\n> }\n>\n> /**\n> @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> */\n> ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> {\n> -\treturn find(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn end();\n> +\n> +\treturn find(iter->second);\n> }\n>\n> /**\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0AFDC6136F\n\tfor ;\n\tFri, 15 Nov 2019 16:58:45 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 652361BF205;\n\tFri, 15 Nov 2019 15:58:44 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 15 Nov 2019 17:00:45 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191115160045.jyqjzpn5eup52vgy@uno.localdomain>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"fj6qrczm3aquznhv\"","Content-Disposition":"inline","In-Reply-To":"<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Fri, 15 Nov 2019 15:58:45 -0000"}},{"id":3039,"web_url":"https://patchwork.libcamera.org/comment/3039/","msgid":"<20191118004454.GK4853@pendragon.ideasonboard.com>","date":"2019-11-18T00:44:54","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, Nov 15, 2019 at 05:00:45PM +0100, Jacopo Mondi wrote:\n> On Fri, Nov 08, 2019 at 10:53:48PM +0200, Laurent Pinchart wrote:\n> > The ControlList count() and find() methods use at() to lookup the\n> > control numerical ID in the idmap_. This causes an exception to be\n> > thrown if the ID doesn't exist in the map. Fix it by using the find()\n> > method instead.\n> \n> Be aware that ControlList::at() (const) still uses at() and could\n> potentially raise an exception as well, while I think we clearly\n> prescribes that libcamera does not uses them.\n\nThere's not much we can do about that, as at() returns a reference, so\nthere's no way to return an error. It's an error in the caller to use\nat() without checking if the item exists first. We could specify this in\nthe documentation, or maybe remove the at() method completely, forcing\nthe caller to use find(). What do you think is best ?\n\n> If that's fine please have mine\n> Reviewed-by: Jacopo Mondi \n> \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/controls.cpp | 18 +++++++++++++++---\n> > 1 file changed, 15 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 93ad2fc6a276..0c7cd449ad64 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> > */\n> > ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > {\n> > -\treturn count(idmap_.at(id));\n> > +\tauto iter = idmap_.find(id);\n> > +\tif (iter == idmap_.end())\n> > +\t\treturn 0;\n> > +\n> > +\treturn count(iter->second);\n> > }\n> >\n> > /**\n> > @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > */\n> > ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > {\n> > -\treturn find(idmap_.at(id));\n> > +\tauto iter = idmap_.find(id);\n> > +\tif (iter == idmap_.end())\n> > +\t\treturn end();\n> > +\n> > +\treturn find(iter->second);\n> > }\n> >\n> > /**\n> > @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > */\n> > ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> > {\n> > -\treturn find(idmap_.at(id));\n> > +\tauto iter = idmap_.find(id);\n> > +\tif (iter == idmap_.end())\n> > +\t\treturn end();\n> > +\n> > +\treturn find(iter->second);\n> > }\n> >\n> > /**","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC8E260C33\n\tfor ;\n\tMon, 18 Nov 2019 01:44:58 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6C168A04;\n\tMon, 18 Nov 2019 01:44:58 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574037898;\n\tbh=6SIFEiHc7c7ualku3YGOAbXRv7/RP5NufIsFts0x/fo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eARSNy2oaTM1EzG5njkaFEJ2NW4cXf1Zl+9OhUKgCufowiy66ymlymyV0HTDwbHLN\n\tFIDuXZli9hzVYSFyYah7OW6OjFHix8hiROlO8sQNcUrFtgFgG734axXmTuRI4/kPYE\n\t8+lJI8O5L7EvE1LAKFcDrSvqTAFg21DMblYVqSzM=","Date":"Mon, 18 Nov 2019 02:44:54 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191118004454.GK4853@pendragon.ideasonboard.com>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>\n\t<20191115160045.jyqjzpn5eup52vgy@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191115160045.jyqjzpn5eup52vgy@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Mon, 18 Nov 2019 00:44:59 -0000"}},{"id":3085,"web_url":"https://patchwork.libcamera.org/comment/3085/","msgid":"<20191118225121.tuakep5a6lf3jd4o@uno.localdomain>","date":"2019-11-18T22:51:21","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Nov 18, 2019 at 02:44:54AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Fri, Nov 15, 2019 at 05:00:45PM +0100, Jacopo Mondi wrote:\n> > On Fri, Nov 08, 2019 at 10:53:48PM +0200, Laurent Pinchart wrote:\n> > > The ControlList count() and find() methods use at() to lookup the\n\ns/ControlList/ControlInfoMap/\n\n> > > control numerical ID in the idmap_. This causes an exception to be\n> > > thrown if the ID doesn't exist in the map. Fix it by using the find()\n> > > method instead.\n> >\n> > Be aware that ControlList::at() (const) still uses at() and could\n> > potentially raise an exception as well, while I think we clearly\n> > prescribes that libcamera does not uses them.\n>\n> There's not much we can do about that, as at() returns a reference, so\n> there's no way to return an error. It's an error in the caller to use\n> at() without checking if the item exists first. We could specify this in\n> the documentation, or maybe remove the at() method completely, forcing\n> the caller to use find(). What do you think is best ?\n>\n\nI don't see users of ControlInfoMap::at() if not the the\nControlInfoMap class itself. I would be fine in dropping it. There are\nusaged of at() on maps in many places on std::map instances though, so\nyou don't want to remove it for consitency, that's fine as well.\n\n> > If that's fine please have mine\n> > Reviewed-by: Jacopo Mondi \n\n> >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/controls.cpp | 18 +++++++++++++++---\n> > > 1 file changed, 15 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > index 93ad2fc6a276..0c7cd449ad64 100644\n> > > --- a/src/libcamera/controls.cpp\n> > > +++ b/src/libcamera/controls.cpp\n> > > @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> > > */\n> > > ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > > {\n> > > -\treturn count(idmap_.at(id));\n> > > +\tauto iter = idmap_.find(id);\n> > > +\tif (iter == idmap_.end())\n> > > +\t\treturn 0;\n> > > +\n> > > +\treturn count(iter->second);\n> > > }\n> > >\n> > > /**\n> > > @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > > */\n> > > ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > > {\n> > > -\treturn find(idmap_.at(id));\n> > > +\tauto iter = idmap_.find(id);\n> > > +\tif (iter == idmap_.end())\n> > > +\t\treturn end();\n> > > +\n> > > +\treturn find(iter->second);\n> > > }\n> > >\n> > > /**\n> > > @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > > */\n> > > ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> > > {\n> > > -\treturn find(idmap_.at(id));\n> > > +\tauto iter = idmap_.find(id);\n> > > +\tif (iter == idmap_.end())\n> > > +\t\treturn end();\n> > > +\n> > > +\treturn find(iter->second);\n> > > }\n> > >\n> > > /**\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 23B3860F1C\n\tfor ;\n\tMon, 18 Nov 2019 23:49:19 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id A765A40004;\n\tMon, 18 Nov 2019 22:49:18 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 18 Nov 2019 23:51:21 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191118225121.tuakep5a6lf3jd4o@uno.localdomain>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>\n\t<20191115160045.jyqjzpn5eup52vgy@uno.localdomain>\n\t<20191118004454.GK4853@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"n3yvddzmalbeo2le\"","Content-Disposition":"inline","In-Reply-To":"<20191118004454.GK4853@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Mon, 18 Nov 2019 22:49:19 -0000"}},{"id":3097,"web_url":"https://patchwork.libcamera.org/comment/3097/","msgid":"<20191119003519.GO5171@pendragon.ideasonboard.com>","date":"2019-11-19T00:35:19","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Nov 18, 2019 at 11:51:21PM +0100, Jacopo Mondi wrote:\n> On Mon, Nov 18, 2019 at 02:44:54AM +0200, Laurent Pinchart wrote:\n> > On Fri, Nov 15, 2019 at 05:00:45PM +0100, Jacopo Mondi wrote:\n> > > On Fri, Nov 08, 2019 at 10:53:48PM +0200, Laurent Pinchart wrote:\n> > > > The ControlList count() and find() methods use at() to lookup the\n> \n> s/ControlList/ControlInfoMap/\n> \n> > > > control numerical ID in the idmap_. This causes an exception to be\n> > > > thrown if the ID doesn't exist in the map. Fix it by using the find()\n> > > > method instead.\n> > >\n> > > Be aware that ControlList::at() (const) still uses at() and could\n> > > potentially raise an exception as well, while I think we clearly\n> > > prescribes that libcamera does not uses them.\n> >\n> > There's not much we can do about that, as at() returns a reference, so\n> > there's no way to return an error. It's an error in the caller to use\n> > at() without checking if the item exists first. We could specify this in\n> > the documentation, or maybe remove the at() method completely, forcing\n> > the caller to use find(). What do you think is best ?\n> \n> I don't see users of ControlInfoMap::at() if not the the\n> ControlInfoMap class itself. I would be fine in dropping it. There are\n> usaged of at() on maps in many places on std::map instances though, so\n> you don't want to remove it for consitency, that's fine as well.\n\nI think there's at least one user in the tests, as compilation failed\nwhen I dropped the method.\n\nWould you like to submit a patch, or do you think this is overkill ?\n\n> > > If that's fine please have mine\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/controls.cpp | 18 +++++++++++++++---\n> > > > 1 file changed, 15 insertions(+), 3 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > > index 93ad2fc6a276..0c7cd449ad64 100644\n> > > > --- a/src/libcamera/controls.cpp\n> > > > +++ b/src/libcamera/controls.cpp\n> > > > @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> > > > */\n> > > > ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > > > {\n> > > > -\treturn count(idmap_.at(id));\n> > > > +\tauto iter = idmap_.find(id);\n> > > > +\tif (iter == idmap_.end())\n> > > > +\t\treturn 0;\n> > > > +\n> > > > +\treturn count(iter->second);\n> > > > }\n> > > >\n> > > > /**\n> > > > @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > > > */\n> > > > ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > > > {\n> > > > -\treturn find(idmap_.at(id));\n> > > > +\tauto iter = idmap_.find(id);\n> > > > +\tif (iter == idmap_.end())\n> > > > +\t\treturn end();\n> > > > +\n> > > > +\treturn find(iter->second);\n> > > > }\n> > > >\n> > > > /**\n> > > > @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > > > */\n> > > > ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> > > > {\n> > > > -\treturn find(idmap_.at(id));\n> > > > +\tauto iter = idmap_.find(id);\n> > > > +\tif (iter == idmap_.end())\n> > > > +\t\treturn end();\n> > > > +\n> > > > +\treturn find(iter->second);\n> > > > }\n> > > >\n> > > > /**","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2DD0F60C12\n\tfor ;\n\tTue, 19 Nov 2019 01:35:33 +0100 (CET)","from pendragon.ideasonboard.com (unknown [38.98.37.142])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7BAA5563;\n\tTue, 19 Nov 2019 01:35:29 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574123732;\n\tbh=YqTE2AM0wAaen09aN9oZA5RR9SMrFsIzZvfwtT2RxhI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=afiQKUy21vjXFX74GfRdtgBdEqL9aJZVSpK+m14Snv64z/8aUQ1UAZLpLnOm04McI\n\tFLNt3Tqo3oRwbZ7Yc+PmFlW4UQ4kxitm0ClDcWlkzY2FqpHY0dt6XENJl6rPdUISQM\n\te/CLq+d/wGe0w7w4rCJ2+AAwu7cNdUhpf7tL50O0=","Date":"Tue, 19 Nov 2019 02:35:19 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191119003519.GO5171@pendragon.ideasonboard.com>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>\n\t<20191115160045.jyqjzpn5eup52vgy@uno.localdomain>\n\t<20191118004454.GK4853@pendragon.ideasonboard.com>\n\t<20191118225121.tuakep5a6lf3jd4o@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191118225121.tuakep5a6lf3jd4o@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Tue, 19 Nov 2019 00:35:33 -0000"}},{"id":3100,"web_url":"https://patchwork.libcamera.org/comment/3100/","msgid":"<4023e4ca-23fd-242a-fe59-f8ee8a0e29c8@ideasonboard.com>","date":"2019-11-19T15:46:12","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 08/11/2019 20:53, Laurent Pinchart wrote:\n> The ControlList count() and find() methods use at() to lookup the\n> control numerical ID in the idmap_. This causes an exception to be\n> thrown if the ID doesn't exist in the map. Fix it by using the find()\n> method instead.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/controls.cpp | 18 +++++++++++++++---\n> 1 file changed, 15 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 93ad2fc6a276..0c7cd449ad64 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> */\n> ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> {\n> -\treturn count(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn 0;\n> +\n> +\treturn count(iter->second);\n\nIs this further count() redundant?\n\nHas the desired item already been 'found' at that point, and thus we can\nreturn '1'?\n\n\nI fear use of std::pair once again makes this hunk fairly horrible to\nread as a standalone code segment. Even more so with the user of auto\niter. (yes, ->second is my ->first pet-peeve)\n\nActually - Looking below, I suspect the call to count is important,\nbecause we might be counting something else.\n\nCan we add a brief comment here or something to make all of this clear?\n\nOr expand the iter->second to a defined type so it's clear /what/ we're\ncounting.\n\n\n--\nKieran\n\n> }\n> \n> /**\n> @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> */\n> ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> {\n> -\treturn find(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn end();\n> +\n> +\treturn find(iter->second);\n> }\n> \n> /**\n> @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> */\n> ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> {\n> -\treturn find(idmap_.at(id));\n> +\tauto iter = idmap_.find(id);\n> +\tif (iter == idmap_.end())\n> +\t\treturn end();\n> +\n> +\treturn find(iter->second);\n> }\n> \n> /**\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C87960BC2\n\tfor ;\n\tTue, 19 Nov 2019 16:46:15 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D8B39311;\n\tTue, 19 Nov 2019 16:46:14 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574178375;\n\tbh=+9/6PhdyVlDlfZf5w0ATucGPgv49JjitoL4hwpCZilc=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=g0DZdhK/3KuPOGMqBicivwuDRaHtwTg7KAYGIpwfYL3hNeZMDAKQP/xViVw5afd7u\n\tBc4F8rt2Gi4DiiQOUE33ICZLcuhQxZF+UiT5YEl7DIl7PzDZjHKegt24an42UYoEfD\n\tQTxvaT4e18rniWbPtLtwsoTtdGIh6rwEHu8HECBk=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<4023e4ca-23fd-242a-fe59-f8ee8a0e29c8@ideasonboard.com>","Date":"Tue, 19 Nov 2019 15:46:12 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.0","MIME-Version":"1.0","In-Reply-To":"<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Tue, 19 Nov 2019 15:46:15 -0000"}},{"id":3115,"web_url":"https://patchwork.libcamera.org/comment/3115/","msgid":"<20191120034854.GA11631@pendragon.ideasonboard.com>","date":"2019-11-20T03:48:54","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Nov 19, 2019 at 03:46:12PM +0000, Kieran Bingham wrote:\n> On 08/11/2019 20:53, Laurent Pinchart wrote:\n> > The ControlList count() and find() methods use at() to lookup the\n> > control numerical ID in the idmap_. This causes an exception to be\n> > thrown if the ID doesn't exist in the map. Fix it by using the find()\n> > method instead.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/controls.cpp | 18 +++++++++++++++---\n> > 1 file changed, 15 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 93ad2fc6a276..0c7cd449ad64 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> > */\n> > ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > {\n> > -\treturn count(idmap_.at(id));\n> > +\tauto iter = idmap_.find(id);\n> > +\tif (iter == idmap_.end())\n> > +\t\treturn 0;\n> > +\n> > +\treturn count(iter->second);\n> \n> Is this further count() redundant?\n> \n> Has the desired item already been 'found' at that point, and thus we can\n> return '1'?\n> \n> I fear use of std::pair once again makes this hunk fairly horrible to\n> read as a standalone code segment. Even more so with the user of auto\n> iter. (yes, ->second is my ->first pet-peeve)\n\n:-) I dislike it as well, but in this case it's nothing we can control.\nidmap_ is an std::unordered_map, and std::unordered_map::value_type is a\nstd::pair. That's the C++ STL API. What we can do is use local variables\nto make this more explicit.\n\n> Actually - Looking below, I suspect the call to count is important,\n> because we might be counting something else.\n\nActually I don't think so. The idmap is generated with one entry per\nentry in ControlInfoMap, with a 1:1 mapping between the two, so I think\nwe can just return 1 here. Or even better,\n\n\treturn idmap_.count(id);\n\n> Can we add a brief comment here or something to make all of this clear?\n> \n> Or expand the iter->second to a defined type so it's clear /what/ we're\n> counting.\n> \n> > }\n> > \n> > /**\n> > @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > */\n> > ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > {\n> > -\treturn find(idmap_.at(id));\n> > +\tauto iter = idmap_.find(id);\n> > +\tif (iter == idmap_.end())\n> > +\t\treturn end();\n> > +\n> > +\treturn find(iter->second);\n> > }\n> > \n> > /**\n> > @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > */\n> > ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> > {\n> > -\treturn find(idmap_.at(id));\n> > +\tauto iter = idmap_.find(id);\n> > +\tif (iter == idmap_.end())\n> > +\t\treturn end();\n> > +\n> > +\treturn find(iter->second);\n> > }\n> > \n> > /**","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D8756136E\n\tfor ;\n\tWed, 20 Nov 2019 04:49:11 +0100 (CET)","from pendragon.ideasonboard.com (unknown [124.219.31.93])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 14220311;\n\tWed, 20 Nov 2019 04:49:06 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574221750;\n\tbh=//649Md2UI3UZd8iwZM0Y/gFzUHOKBEBdb4DXuouov0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YDhuh1GwoN51elnX7v3FSRXSuBblvzRwwnJ8lbbDE76sa9Fgx9jSUZg5Va9OLbTbJ\n\tEY8ZYSeFYoXgI8borlPMVe1aG7dYwGDSCIJzITYzPAN8XgpVFajxl3araeNyUcGxyb\n\tuVMjOeQxrytG+G7XDqnn1zEfUCdlthDrqb0/0k3E=","Date":"Wed, 20 Nov 2019 05:48:54 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191120034854.GA11631@pendragon.ideasonboard.com>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>\n\t<4023e4ca-23fd-242a-fe59-f8ee8a0e29c8@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<4023e4ca-23fd-242a-fe59-f8ee8a0e29c8@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Wed, 20 Nov 2019 03:49:11 -0000"}},{"id":3117,"web_url":"https://patchwork.libcamera.org/comment/3117/","msgid":"","date":"2019-11-20T09:17:05","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 20/11/2019 03:48, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Tue, Nov 19, 2019 at 03:46:12PM +0000, Kieran Bingham wrote:\n>> On 08/11/2019 20:53, Laurent Pinchart wrote:\n>>> The ControlList count() and find() methods use at() to lookup the\n>>> control numerical ID in the idmap_. This causes an exception to be\n>>> thrown if the ID doesn't exist in the map. Fix it by using the find()\n>>> method instead.\n>>>\n>>> Signed-off-by: Laurent Pinchart \n>>> ---\n>>> src/libcamera/controls.cpp | 18 +++++++++++++++---\n>>> 1 file changed, 15 insertions(+), 3 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>>> index 93ad2fc6a276..0c7cd449ad64 100644\n>>> --- a/src/libcamera/controls.cpp\n>>> +++ b/src/libcamera/controls.cpp\n>>> @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n>>> */\n>>> ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n>>> {\n>>> -\treturn count(idmap_.at(id));\n>>> +\tauto iter = idmap_.find(id);\n>>> +\tif (iter == idmap_.end())\n>>> +\t\treturn 0;\n>>> +\n>>> +\treturn count(iter->second);\n>>\n>> Is this further count() redundant?\n>>\n>> Has the desired item already been 'found' at that point, and thus we can\n>> return '1'?\n>>\n>> I fear use of std::pair once again makes this hunk fairly horrible to\n>> read as a standalone code segment. Even more so with the user of auto\n>> iter. (yes, ->second is my ->first pet-peeve)\n> \n> :-) I dislike it as well, but in this case it's nothing we can control.\n> idmap_ is an std::unordered_map, and std::unordered_map::value_type is a\n> std::pair. That's the C++ STL API. What we can do is use local variables\n> to make this more explicit.\n\nYes, the use of local variables to make the naming explicit is what I\nmeant here ...\n\nI feel like any use of ->first and ->second is the equivalent of naming\nvariables\n int integer = 0;\n\nJust like the std::pair, here there is an integer, but it doesn't tell\nme what it stores it for, The ->first ->second /are/ the first and\nsecond parameter of the pair - but that doesn't tell me what they are\nused for. And it leaves the reader blind (or worse, confused).\n\nI feel strongly enough about this to say we should propose a coding\nstandard addition to say all use of std::pair should be named explicitly\nby mapping to typed (and well named) local variables (which I expect the\ncompiler to optimise out), with perhaps an exception if the std::pair\nreturn types are already exposed within the local function and \"might\"\nbe redundant.\n\nWhat do you think ?\n\n>> Actually - Looking below, I suspect the call to count is important,\n>> because we might be counting something else.\n> \n> Actually I don't think so. The idmap is generated with one entry per\n> entry in ControlInfoMap, with a 1:1 mapping between the two, so I think\n> we can just return 1 here. Or even better,\n> \n> \treturn idmap_.count(id);\n\n\nThat sounds even better.\nPerhaps something like this then (Adjust as appropriate of course):\n\nControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n{\n /*\n * The idmap is generated with one entry per entry in ControlInfoMap\n * with a 1:1 mapping between the two, therefore we count the relevant\n * idmap entries to establish the ControlInfoMap count without\n * performing an additional lookup.\n */\n\n return idmap_.count(id);\n}\n\n> \n>> Can we add a brief comment here or something to make all of this clear?\n>>\n>> Or expand the iter->second to a defined type so it's clear /what/ we're\n>> counting.\n>>\n>>> }\n>>> \n>>> /**\n>>> @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n>>> */\n>>> ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n>>> {\n>>> -\treturn find(idmap_.at(id));\n>>> +\tauto iter = idmap_.find(id);\n>>> +\tif (iter == idmap_.end())\n>>> +\t\treturn end();\n>>> +\n>>> +\treturn find(iter->second);\n>>> }\n>>> \n>>> /**\n>>> @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n>>> */\n>>> ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n>>> {\n>>> -\treturn find(idmap_.at(id));\n>>> +\tauto iter = idmap_.find(id);\n>>> +\tif (iter == idmap_.end())\n>>> +\t\treturn end();\n>>> +\n>>> +\treturn find(iter->second);\n>>> }\n>>> \n>>> /**\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7886860C22\n\tfor ;\n\tWed, 20 Nov 2019 10:17:08 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BF7FD311;\n\tWed, 20 Nov 2019 10:17:07 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574241428;\n\tbh=RZcvqQmapuieGMd+Qt8i9M/RMUlEgL+Qy0MJsz10IJs=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Y/Df3GhOCLtVZjkXKHUL4FJI8p39bP2hc+S1cDxbakMZ9K9PmnD0YB7cXsizJQIZ9\n\to+sFDwtD2rQs1X20CyFrew/1xWbe3qWy7XNbbc2bUXbIACctzSSRIy4RhwiFcIXyMV\n\t8dgmLdKb+L52N3VJsxLv3RnnliqGZqZ2H014IX/o=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>\n\t<4023e4ca-23fd-242a-fe59-f8ee8a0e29c8@ideasonboard.com>\n\t<20191120034854.GA11631@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Wed, 20 Nov 2019 09:17:05 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.0","MIME-Version":"1.0","In-Reply-To":"<20191120034854.GA11631@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Wed, 20 Nov 2019 09:17:08 -0000"}},{"id":3124,"web_url":"https://patchwork.libcamera.org/comment/3124/","msgid":"<20191121074603.GG4958@pendragon.ideasonboard.com>","date":"2019-11-21T07:46:03","subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Nov 20, 2019 at 09:17:05AM +0000, Kieran Bingham wrote:\n> On 20/11/2019 03:48, Laurent Pinchart wrote:\n> > On Tue, Nov 19, 2019 at 03:46:12PM +0000, Kieran Bingham wrote:\n> >> On 08/11/2019 20:53, Laurent Pinchart wrote:\n> >>> The ControlList count() and find() methods use at() to lookup the\n> >>> control numerical ID in the idmap_. This causes an exception to be\n> >>> thrown if the ID doesn't exist in the map. Fix it by using the find()\n> >>> method instead.\n> >>>\n> >>> Signed-off-by: Laurent Pinchart \n> >>> ---\n> >>> src/libcamera/controls.cpp | 18 +++++++++++++++---\n> >>> 1 file changed, 15 insertions(+), 3 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> >>> index 93ad2fc6a276..0c7cd449ad64 100644\n> >>> --- a/src/libcamera/controls.cpp\n> >>> +++ b/src/libcamera/controls.cpp\n> >>> @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> >>> */\n> >>> ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> >>> {\n> >>> -\treturn count(idmap_.at(id));\n> >>> +\tauto iter = idmap_.find(id);\n> >>> +\tif (iter == idmap_.end())\n> >>> +\t\treturn 0;\n> >>> +\n> >>> +\treturn count(iter->second);\n> >>\n> >> Is this further count() redundant?\n> >>\n> >> Has the desired item already been 'found' at that point, and thus we can\n> >> return '1'?\n> >>\n> >> I fear use of std::pair once again makes this hunk fairly horrible to\n> >> read as a standalone code segment. Even more so with the user of auto\n> >> iter. (yes, ->second is my ->first pet-peeve)\n> > \n> > :-) I dislike it as well, but in this case it's nothing we can control.\n> > idmap_ is an std::unordered_map, and std::unordered_map::value_type is a\n> > std::pair. That's the C++ STL API. What we can do is use local variables\n> > to make this more explicit.\n> \n> Yes, the use of local variables to make the naming explicit is what I\n> meant here ...\n> \n> I feel like any use of ->first and ->second is the equivalent of naming\n> variables\n> int integer = 0;\n> \n> Just like the std::pair, here there is an integer, but it doesn't tell\n> me what it stores it for, The ->first ->second /are/ the first and\n> second parameter of the pair - but that doesn't tell me what they are\n> used for. And it leaves the reader blind (or worse, confused).\n> \n> I feel strongly enough about this to say we should propose a coding\n> standard addition to say all use of std::pair should be named explicitly\n> by mapping to typed (and well named) local variables (which I expect the\n> compiler to optimise out), with perhaps an exception if the std::pair\n> return types are already exposed within the local function and \"might\"\n> be redundant.\n> \n> What do you think ?\n\nI agree with you, and we're already doing this in most cases. I would\nhowever relax the rule a little bit, in some cases using .first and\n.second is explicit enough, but that's far from being the general rule.\n\nOne case where .first and .second is fine in my opinion is when they\nreally denote the first and second instances of something, with an\nexplicit std::pair.\n\n\t/* French fries have to be fried twice. */\n\tstd::pair cookingDuration;\n\tfry(fries, cookingTime.first);\n\tstd::thread::wait_for(restDuration);\n\tfry(fries, cookingTime.second);\n\nAnother case could be when the map type is explicitly shown in the code\niterating on the map. I find the following explicit enough:\n\n\tstd::map theMap = generateTheMap();\n\n\tfor (const auto &pair : theMap)\n\t\tstd::cout << \"mapping from \" << pair.first\n\t\t\t << \" to \" << pair.second << std::endl;\n\nBut this wouldn't be explicit enough:\n\n\tfor (const auto &pair : theMap_)\n\t\tstd::cout << \"mapping from \" << pair.first\n\t\t\t << \" to \" << pair.second << std::endl;\n\nas the definition of theMap_ is located in a different place.\n\n> >> Actually - Looking below, I suspect the call to count is important,\n> >> because we might be counting something else.\n> > \n> > Actually I don't think so. The idmap is generated with one entry per\n> > entry in ControlInfoMap, with a 1:1 mapping between the two, so I think\n> > we can just return 1 here. Or even better,\n> > \n> > \treturn idmap_.count(id);\n> \n> That sounds even better.\n> Perhaps something like this then (Adjust as appropriate of course):\n> \n> ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> {\n> /*\n> * The idmap is generated with one entry per entry in ControlInfoMap\n> * with a 1:1 mapping between the two, therefore we count the relevant\n> * idmap entries to establish the ControlInfoMap count without\n> * performing an additional lookup.\n> */\n> \n> return idmap_.count(id);\n> }\n\nI've applied a slightly modified version. Thank you for the proposal.\n\n> >> Can we add a brief comment here or something to make all of this clear?\n> >>\n> >> Or expand the iter->second to a defined type so it's clear /what/ we're\n> >> counting.\n> >>\n> >>> }\n> >>> \n> >>> /**\n> >>> @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> >>> */\n> >>> ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> >>> {\n> >>> -\treturn find(idmap_.at(id));\n> >>> +\tauto iter = idmap_.find(id);\n> >>> +\tif (iter == idmap_.end())\n> >>> +\t\treturn end();\n> >>> +\n> >>> +\treturn find(iter->second);\n> >>> }\n> >>> \n> >>> /**\n> >>> @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> >>> */\n> >>> ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> >>> {\n> >>> -\treturn find(idmap_.at(id));\n> >>> +\tauto iter = idmap_.find(id);\n> >>> +\tif (iter == idmap_.end())\n> >>> +\t\treturn end();\n> >>> +\n> >>> +\treturn find(iter->second);\n> >>> }\n> >>> \n> >>> /**","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F18546136C\n\tfor ;\n\tThu, 21 Nov 2019 08:46:12 +0100 (CET)","from pendragon.ideasonboard.com (unknown [124.219.31.93])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E7ED9A2B;\n\tThu, 21 Nov 2019 08:46:11 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574322372;\n\tbh=ImAxRoWHYseID3mcluCWvRD3N8Q2w0k3PAV9/0zQx8M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FeBU0+xWXHR02+9N0jc2MT7/ZJal6yDS3z0hrf6cZUoqX0EDlZbTxQWK8cXoobr0j\n\t9/B5Lny86PrJ0mJY5HqtMEJ/HarJYy/A7aTD1IzMxDrVUhOyRMQSJmNoJ8guNW4lKO\n\tBosy3nlTYva/8ePRIF8LrC2ViTSTHN4tbiv0DorY=","Date":"Thu, 21 Nov 2019 09:46:03 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191121074603.GG4958@pendragon.ideasonboard.com>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-4-laurent.pinchart@ideasonboard.com>\n\t<4023e4ca-23fd-242a-fe59-f8ee8a0e29c8@ideasonboard.com>\n\t<20191120034854.GA11631@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid\n\texception in ControlList count() and find()","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","X-List-Received-Date":"Thu, 21 Nov 2019 07:46:13 -0000"}}]