[{"id":24335,"web_url":"https://patchwork.libcamera.org/comment/24335/","msgid":"<Yupd+Eu8DfHMeVZL@pendragon.ideasonboard.com>","date":"2022-08-03T11:37:28","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: yaml_parser: Return\n\tnullopt on error","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 Wed, Aug 03, 2022 at 01:29:37PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> The YamlParser::getList<>() function returns an std::optional<> to allow\n> callers to identify cases where parsing the .yaml file failed from cases\n> where the parsed list is just empty.\n> \n> The current implementation returns an empty vector in case of parsing\n> errors, which evaluates to std::nullopt but it less explicit.\n\n{} is not an empty vector, as it's not a vector :-)\n\n\nThe current implementation returns a default-constructed std::optional\nin case of errors with\n\n\treturn {};\n\nThe returned value is thus equal to std::nullopt, but the code can be\neasily misinterpreted as returning an empty vector by a reader.\n\n\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> Change this by returning std::nullopt in case of errors to make the\n> intended behaviour more explicit.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>  src/libcamera/yaml_parser.cpp | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> index 84cb57d6de83..c96e99e1317c 100644\n> --- a/src/libcamera/yaml_parser.cpp\n> +++ b/src/libcamera/yaml_parser.cpp\n> @@ -319,7 +319,7 @@ template<typename T,\n>  std::optional<std::vector<T>> YamlObject::getList() const\n>  {\n>  \tif (type_ != Type::List)\n> -\t\treturn {};\n> +\t\treturn std::nullopt;\n>  \n>  \tstd::vector<T> values;\n>  \tvalues.reserve(list_.size());\n> @@ -327,7 +327,7 @@ std::optional<std::vector<T>> YamlObject::getList() const\n>  \tfor (const YamlObject &entry : asList()) {\n>  \t\tconst auto value = entry.get<T>();\n>  \t\tif (!value)\n> -\t\t\treturn {};\n> +\t\t\treturn std::nullopt;\n>  \t\tvalues.emplace_back(*value);\n>  \t}\n>","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 7564CC3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Aug 2022 11:37:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C52FC63310;\n\tWed,  3 Aug 2022 13:37:35 +0200 (CEST)","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 78C20603E6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Aug 2022 13:37:34 +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 F0A598B;\n\tWed,  3 Aug 2022 13:37:33 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659526655;\n\tbh=cAaInzyG4ecUgF7GnrHDo9A9sfleZclmA9Ku3XP9HIw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=y72srvCGYVCdBjgEJ8tgmm24ar/T7zsHyTKrHiM93dM58s+FBcs0CKQjeNx/Bc/MD\n\ttheOa9WUp2jpf2l3UTBihEKk1oE11w+4O0dXPdyNbZDeD3tiMdnd3Bu9XwtfvdxIN2\n\t60awvft6GExJmZel7Mw5tpsAjNFk+tW2syXwAb/IMeLW/eWF67zNDZ+2CKYl5OLyNg\n\tc8559luTEtRFMPGp6Z7NOympqsWpN2KS2qhTvhwVgmDVm/mGe9emLcM2qy29QZoNg8\n\twfOO6Rz9eXPhxb7TSy4ohwzx6tM+afOsZmPAy9kUsfjSPTUnSsegYb6JS1BZIVUcTy\n\t8M36saywRJUlw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659526654;\n\tbh=cAaInzyG4ecUgF7GnrHDo9A9sfleZclmA9Ku3XP9HIw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ryO3hNzNo6Jdjkhtft09itfZAtEnQx0dW+iHOLTz/uAiemG0lI7Ym1FmDoridaSp9\n\tyzVI40abjf6P35H5JiQBeGLNJ9Yvqeul5n7rZNKMuPEXmxstmA/oduHIA/fryitvWM\n\t7hpSF7l4YonvdOZDVw5EuwPMeP+cxMbo2A63cIek="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ryO3hNzN\"; dkim-atps=neutral","Date":"Wed, 3 Aug 2022 14:37:28 +0300","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<Yupd+Eu8DfHMeVZL@pendragon.ideasonboard.com>","References":"<20220803112937.30566-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220803112937.30566-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: yaml_parser: Return\n\tnullopt on error","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]