[{"id":714,"web_url":"https://patchwork.libcamera.org/comment/714/","msgid":"<20190131095040.GC4197@pendragon.ideasonboard.com>","date":"2019-01-31T09:50:40","subject":"Re: [libcamera-devel] [PATCH 2/6] cam: options: create a template\n\tclass for options","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Jan 28, 2019 at 01:41:05AM +0100, Niklas Söderlund wrote:\n> In preparation to adding more parsers create a template class to hold\n> the parsed information. The rational for making it a template are that\n> different parsers can index the options using different data types.\n> \n> The OptionsParser index its options using an int while the upcoming\n> KeyValyeParser will index its options using strings for example.\n\ns/Valye/Value/\n\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/cam/options.cpp | 24 ------------------------\n> src/cam/options.h | 28 ++++++++++++++++------------\n> 2 files changed, 16 insertions(+), 36 deletions(-)\n> \n> diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> index 83601270207b67b3..b24964a8ce413a85 100644\n> --- a/src/cam/options.cpp\n> +++ b/src/cam/options.cpp\n> @@ -166,27 +166,3 @@ void OptionsParser::usage()\n> \t\t}\n> \t}\n> }\n> -\n> -OptionsParser::Options::Options()\n> -{\n> -}\n> -\n> -bool OptionsParser::Options::valid() const\n> -{\n> -\treturn !values_.empty();\n> -}\n> -\n> -bool OptionsParser::Options::isSet(int opt) const\n> -{\n> -\treturn values_.find(opt) != values_.end();\n> -}\n> -\n> -const std::string &OptionsParser::Options::operator[](int opt) const\n> -{\n> -\treturn values_.find(opt)->second;\n> -}\n> -\n> -void OptionsParser::Options::clear()\n> -{\n> -\tvalues_.clear();\n> -}\n> diff --git a/src/cam/options.h b/src/cam/options.h\n> index 491f6a316fffbe5b..a08bfea1ba74c96b 100644\n> --- a/src/cam/options.h\n> +++ b/src/cam/options.h\n> @@ -17,21 +17,25 @@ enum OptionArgument {\n> \tArgumentOptional,\n> };\n> \n> +template \n> +class OptionsBase\n> +{\n> +public:\n> +\tbool valid() const { return !values_.empty(); };\n> +\tbool isSet(T opt) const { return values_.find(opt) != values_.end(); };\n> +\tconst std::string &operator[](T opt) const { return values_.find(opt)->second; };\n> +\n> +private:\n> +\tfriend class OptionsParser;\n> +\tstd::map values_;\n> +\tvoid clear() { values_.clear(); };\n> +};\n\nOne common issue with templates is how they push for inlining methods.\nThis is however not required when all the instantiations of the template\nclass are known in advance. In this specific case, you can for instance\napply the following change on top of this patch to avoid inlining\nmethods:\n\ndiff --git a/src/cam/options.cpp b/src/cam/options.cpp\nindex f63fd3b495e2..3f4781ae96bb 100644\n--- a/src/cam/options.cpp\n+++ b/src/cam/options.cpp\n@@ -12,6 +12,33 @@\n\n #include \"options.h\"\n\n+template \n+bool OptionsBase::valid() const\n+{\n+\treturn !values_.empty();\n+}\n+\n+template \n+bool OptionsBase::isSet(T opt) const\n+{\n+\treturn values_.find(opt) != values_.end();\n+}\n+\n+template \n+const std::string &OptionsBase::operator[](T opt) const\n+{\n+\treturn values_.find(opt)->second;\n+}\n+\n+template \n+void OptionsBase::clear()\n+{\n+\tvalues_.clear();\n+}\n+\n+template class OptionsBase;\n+template class OptionsBase;\n+\n bool OptionsParser::addOption(int opt, const char *help, const char *name,\n \t\t\t OptionArgument argument, const char *argumentName)\n {\ndiff --git a/src/cam/options.h b/src/cam/options.h\nindex 91a78406a601..dff0d4f4ca22 100644\n--- a/src/cam/options.h\n+++ b/src/cam/options.h\n@@ -21,15 +21,15 @@ template \n class OptionsBase\n {\n public:\n-\tbool valid() const { return !values_.empty(); };\n-\tbool isSet(T opt) const { return values_.find(opt) != values_.end(); };\n-\tconst std::string &operator[](T opt) const { return values_.find(opt)->second; };\n+\tbool valid() const;\n+\tbool isSet(T opt) const;\n+\tconst std::string &operator[](T opt) const;\n\n private:\n \tfriend class OptionsParser;\n \tfriend class KeyValueParser;\n \tstd::map values_;\n-\tvoid clear() { values_.clear(); };\n+\tvoid clear();\n };\n\n class KeyValueParser\n\nThis isn't mandatory but may help reducing compilation time and memory\nfootprint when the cam application will grow.\n\nReviewed-by: Laurent Pinchart \n\n> class OptionsParser\n> {\n> public:\n> -\tclass Options {\n> -\tpublic:\n> -\t\tOptions();\n> -\n> -\t\tbool valid() const;\n> -\t\tbool isSet(int opt) const;\n> -\t\tconst std::string &operator[](int opt) const;\n> -\n> -\tprivate:\n> -\t\tfriend class OptionsParser;\n> -\t\tstd::map values_;\n> -\t\tvoid clear();\n> +\tclass Options : public OptionsBase\n> +\t{\n> \t};\n> \n> \tvoid addOption(int opt, const char *help, const char *name = nullptr,","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 6E74860B10\n\tfor ;\n\tThu, 31 Jan 2019 10:50:46 +0100 (CET)","from pendragon.ideasonboard.com (85-76-34-136-nat.elisa-mobile.fi\n\t[85.76.34.136])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8B25441;\n\tThu, 31 Jan 2019 10:50:44 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548928246;\n\tbh=88IytRcZlvb4pyELX5WYI4Kqgb/2mhSg9wXwLony0YU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=G3EcatTxqMg17doRTfYgtsZUOxeFxQqNyiF+vYm6HLEEal/wAyJK+EMDG84TdiDRR\n\tFiYCgbByMR+VmuSlMhtHw+pRYtkXMFyU21Uj9u8kpUipbhQt+MvNr3lb1/HqJMTsDB\n\t53WN7IZbWBG92pK3grVnYtGUKb0xz3tsTnRBVyvY=","Date":"Thu, 31 Jan 2019 11:50:40 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190131095040.GC4197@pendragon.ideasonboard.com>","References":"<20190128004109.25860-1-niklas.soderlund@ragnatech.se>\n\t<20190128004109.25860-3-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190128004109.25860-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/6] cam: options: create a template\n\tclass for options","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 31 Jan 2019 09:50:46 -0000"}}]