[{"id":32559,"web_url":"https://patchwork.libcamera.org/comment/32559/","msgid":"<20241206003217.GK21014@pendragon.ideasonboard.com>","date":"2024-12-06T00:32:17","subject":"Re: [PATCH v1 1/3] libcamera: yaml_parser: Add\n\t`YamlObject::find(key)`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Thu, Dec 05, 2024 at 04:34:14PM +0000, Barnabás Pőcze wrote:\n> This function is meant to replace `YamlObject::contains()`\n> as it can be easily used for the same purpose, i.e. determining\n> if a certain key exists in a dictionary, but it has the advantage\n> that the result, if any, is immediately available for use\n> without having to do a second lookup.\n> \n> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> ---\n>  include/libcamera/internal/yaml_parser.h |  1 +\n>  src/libcamera/yaml_parser.cpp            | 23 +++++++++++++++++++++++\n>  2 files changed, 24 insertions(+)\n> \n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index 8c791656..796e3e90 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -209,6 +209,7 @@ public:\n>  \n>  \tbool contains(std::string_view key) const;\n>  \tconst YamlObject &operator[](std::string_view key) const;\n> +\tconst YamlObject *find(std::string_view key) const;\n>  \n>  private:\n>  \tLIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject)\n> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> index db256ec5..da8cb61f 100644\n> --- a/src/libcamera/yaml_parser.cpp\n> +++ b/src/libcamera/yaml_parser.cpp\n> @@ -397,6 +397,29 @@ const YamlObject &YamlObject::operator[](std::string_view key) const\n>  \treturn *iter->second;\n>  }\n>  \n> +/**\n> + * \\fn YamlObject::find(std::string_view key) const\n\nYou can drop this, not required when the documentation block directly\nprecedes the function.\n\n> + * \\brief Retrieve a member by name from the dictionary\n\nMissing \\param\n\n> + *\n> + * This function retrieves a member of a YamlObject by name. Only YamlObject\n> + * instances of Dictionary type associate elements with names, calling this\n> + * function on other types of instances or with a nonexistent key results in\n> + * \\a nullptr being returned.\n> + *\n> + * \\return The YamlObject corresponding to the \\a key member\n> + */\n> +const YamlObject *YamlObject::find(std::string_view key) const\n> +{\n> +\tif (type_ != Type::Dictionary)\n> +\t\treturn nullptr;\n> +\n> +\tauto iter = dictionary_.find(key);\n> +\tif (iter == dictionary_.end())\n> +\t\treturn nullptr;\n> +\n> +\treturn iter->second;\n> +}\n\nThe implementation looks fine. Assuming there's no issue with the way\nthe function is used in subsequent patches,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\n>  #ifndef __DOXYGEN__\n>  \n>  class YamlParserContext","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 0B592BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Dec 2024 00:32:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 499A36611D;\n\tFri,  6 Dec 2024 01:32:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F0786611A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Dec 2024 01:32:30 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC5A116A;\n\tFri,  6 Dec 2024 01:32:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gSYcAWEG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733445121;\n\tbh=A2UiP2tNUv541W54xYm72LCJ9Las/FoP25dab/iT3Mk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gSYcAWEG/sORcmooGQRhMlCcb3pY+YsdrzReQoIMC+4wQ+g2DCpJ9zl5gkrt7UbHR\n\tHZP3JoxA/Mre5Sxze91Z1p/XDt2EDqgrmcnGkuTsRZAeRd9ljt+S481kdLxOyZFV/x\n\tEnQuh+jnK5lezlFR37B2V3RlaS/Zvl4zgY9p8TJA=","Date":"Fri, 6 Dec 2024 02:32:17 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/3] libcamera: yaml_parser: Add\n\t`YamlObject::find(key)`","Message-ID":"<20241206003217.GK21014@pendragon.ideasonboard.com>","References":"<20241205163411.1160094-1-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20241205163411.1160094-1-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]