[{"id":31255,"web_url":"https://patchwork.libcamera.org/comment/31255/","msgid":"","date":"2024-09-17T13:45:13","subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. szeptember 17., kedd 00:28 keltezéssel, Stefan Klug írta:\n\n> When accessing a nonexistent key on a dict the YamlObject returns an\n> empty element. This element can happily be casted to a string which is\n> unexpected. For example the following statement:\n> yamlDict[\"nonexistant\"].get(\"default\") is expected to return\n> \"default\" but actually returns \"\". Fix this by introducing a empty type\n> to distinguish between an empty YamlObject and a YamlObject of type\n> value containing an empty string. For completeness add an isEmpty()\n> function to be able to test for that type.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> include/libcamera/internal/yaml_parser.h | 5 +++++\n> src/libcamera/yaml_parser.cpp | 20 ++++++++++++++------\n> test/yaml-parser.cpp | 6 ++++++\n> 3 files changed, 25 insertions(+), 6 deletions(-)\n> \n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index 16708e488d88..33c3a96fed47 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -159,6 +159,10 @@ public:\n> \t{\n> \t\treturn type_ == Type::Dictionary;\n> \t}\n> +\tbool isEmpty() const\n> +\t{\n> +\t\treturn type_ == Type::Empty;\n> +\t}\n\nI think `constexpr explicit operator bool() const` would be a good addition as well.\nThen you can just do e.g.\n\n if (auto y = x[\"something\"]) {\n // ...\n }\n\n\n> \n> \tstd::size_t size() const;\n> \n> @@ -212,6 +216,7 @@ private:\n> \t\tDictionary,\n> \t\tList,\n> \t\tValue,\n> +\t\tEmpty,\n> \t};\n> [...]\n\n\nRegards,\nBarnabás Pőcze","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 A2054C324C\n\tfor ;\n\tTue, 17 Sep 2024 13:45:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7AFEF634FD;\n\tTue, 17 Sep 2024 15:45:22 +0200 (CEST)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F3D0A618F1\n\tfor ;\n\tTue, 17 Sep 2024 15:45:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"VOvA1t3L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1726580719; x=1726839919;\n\tbh=Ne1rp7UkT8wcvcJLv5/mdXqBbmESK7eYHEuETNljpI8=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=VOvA1t3LZcr5LYv5WJ6bCy+TSvDPYV9/GHRd/cBw+cu3zQuyhpy0JutDkC1JR5YAH\n\tnqRZF0CK/FWFJAYd+H4o37ZGNkQV7+waqVs4uLrEbkGbHk6lkhSsxZPEERB+F+LGuw\n\tMUTb0X3rqPKA5dUT5WBQHjvi3u6qhQv3D9LohqvlTSNGmIvDOy32Keg//sAwu0QI+2\n\tBtIAV1ibzJMe6jNcikW3EmDiljep0ZX2wSwFKIpK45yaJtSVPvFyt09JesbJRNkdwx\n\tGD64jU8Svi7HtzmK6r86eME4gqxA1eK/Ba8QIi5vm9byzqOAkFVXmzOWN0Sg0rv4+n\n\tCnKTaRsse4aQg==","Date":"Tue, 17 Sep 2024 13:45:13 +0000","To":"Stefan Klug ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","Message-ID":"","In-Reply-To":"<20240916222834.464460-1-stefan.klug@ideasonboard.com>","References":"<20240916222834.464460-1-stefan.klug@ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"61dc3e740bd757f9aef5acccdbd0b234f1e99895","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31256,"web_url":"https://patchwork.libcamera.org/comment/31256/","msgid":"","date":"2024-09-17T14:12:45","subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Sep 17, 2024 at 12:28:20AM +0200, Stefan Klug wrote:\n> When accessing a nonexistent key on a dict the YamlObject returns an\n> empty element. This element can happily be casted to a string which is\n> unexpected. For example the following statement:\n> yamlDict[\"nonexistant\"].get(\"default\") is expected to return\n\ns/nonexistant/nonexistent/\n\n> \"default\" but actually returns \"\". Fix this by introducing a empty type\n\ns/ a / an /\n\n> to distinguish between an empty YamlObject and a YamlObject of type\n> value containing an empty string. For completeness add an isEmpty()\n> function to be able to test for that type.\n\nOoh that's a smart solution.\n\n> \n> Signed-off-by: Stefan Klug \n> ---\n> include/libcamera/internal/yaml_parser.h | 5 +++++\n> src/libcamera/yaml_parser.cpp | 20 ++++++++++++++------\n> test/yaml-parser.cpp | 6 ++++++\n> 3 files changed, 25 insertions(+), 6 deletions(-)\n> \n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index 16708e488d88..33c3a96fed47 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -159,6 +159,10 @@ public:\n> \t{\n> \t\treturn type_ == Type::Dictionary;\n> \t}\n> +\tbool isEmpty() const\n> +\t{\n> +\t\treturn type_ == Type::Empty;\n> +\t}\n> \n> \tstd::size_t size() const;\n> \n> @@ -212,6 +216,7 @@ private:\n> \t\tDictionary,\n> \t\tList,\n> \t\tValue,\n> +\t\tEmpty,\n> \t};\n> \n> \ttemplate\n> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> index 8b6a403898be..eb72529136f7 100644\n> --- a/src/libcamera/yaml_parser.cpp\n> +++ b/src/libcamera/yaml_parser.cpp\n> @@ -38,12 +38,12 @@ static const YamlObject empty;\n> * \\brief A class representing the tree structure of the YAML content\n> *\n> * The YamlObject class represents the tree structure of YAML content. A\n> - * YamlObject can be a dictionary or list of YamlObjects or a value if a tree\n> - * leaf.\n> + * YamlObject can be empty, a dictionary or list of YamlObjects or a value if a\n> + * tree leaf.\n> */\n> \n> YamlObject::YamlObject()\n> -\t: type_(Type::Value)\n> +\t: type_(Type::Empty)\n> {\n> }\n> \n> @@ -70,6 +70,13 @@ YamlObject::~YamlObject() = default;\n> * \\return True if the YamlObject is a dictionary, false otherwise\n> */\n> \n> +/**\n> + * \\fn YamlObject::isEmpty()\n> + * \\brief Return whether the YamlObject is a empty\n\ns/ a / an /\n\n> + *\n> + * \\return True if the YamlObject is empty, false otherwise\n> + */\n> +\n> /**\n> * \\fn YamlObject::size()\n> * \\brief Retrieve the number of elements in a dictionary or list YamlObject\n> @@ -443,7 +450,8 @@ template std::optional> YamlObject::getList() const;\n> *\n> * This function retrieves an element of the YamlObject. Only YamlObject\n> * instances of List type associate elements with index, calling this function\n> - * on other types of instances is invalid and results in undefined behaviour.\n> + * on other types of instances or with an invalid index results in an empty\n> + * object.\n> *\n> * \\return The YamlObject as an element of the list\n> */\n> @@ -480,8 +488,8 @@ bool YamlObject::contains(const std::string &key) const\n> *\n> * This function retrieve 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 is invalid and results in undefined\n> - * behaviour.\n> + * function on other types of instances or with a non existant key results in an\n\ns/non existant/nonexistent/\n\nReviewed-by: Paul Elder \n\n> + * empty object.\n> *\n> * \\return The YamlObject corresponding to the \\a key member\n> */\n> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> index 81c829834667..b8ae0b4c11f8 100644\n> --- a/test/yaml-parser.cpp\n> +++ b/test/yaml-parser.cpp\n> @@ -514,6 +514,12 @@ protected:\n> \t\t\ti++;\n> \t\t}\n> \n> +\t\t/* Test access to nonexistent member */\n> +\t\tif (dictObj[\"nonexistent\"].get(\"default\") != \"default\") {\n> +\t\t\tcerr << \"Accessing nonexistent dict fails to return default\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\t/* Make sure utils::map_keys() works on the adapter. */\n> \t\t(void)utils::map_keys(dictObj.asDict());\n> \n> -- \n> 2.43.0\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 4C2B9C3257\n\tfor ;\n\tTue, 17 Sep 2024 14:12:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 57AC3634F8;\n\tTue, 17 Sep 2024 16:12:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E656634F8\n\tfor ;\n\tTue, 17 Sep 2024 16:12:50 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2001:4bc9:a45:b0af:31d1:7a87:7f90:977b])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 29376514;\n\tTue, 17 Sep 2024 16:11:28 +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=\"iMwK4LlU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726582288;\n\tbh=91xMdc/ZtwGfT7DX+y9LYNssxfjvLJgKfWb8GUoTH/w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iMwK4LlUtOYhCHa0NSeZahc9T+asGE+F0OZRA+qsY2f70oRo/1CIdZ7YzZDqbWjcT\n\tNM4vJRK2FWKWcUSt1iypGQZKEoS/gaqNTBLK9/jXnEdnsigs6nYZOfxofT1ifoL6/C\n\t+qM0WQfyZl438RppPhwKWdje76jJVGBigZcEZVlE=","Date":"Tue, 17 Sep 2024 16:12:45 +0200","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","Message-ID":"","References":"<20240916222834.464460-1-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240916222834.464460-1-stefan.klug@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31257,"web_url":"https://patchwork.libcamera.org/comment/31257/","msgid":"<20240917150938.GF17350@pendragon.ideasonboard.com>","date":"2024-09-17T15:09:38","subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Sep 17, 2024 at 04:12:45PM +0200, Paul Elder wrote:\n> On Tue, Sep 17, 2024 at 12:28:20AM +0200, Stefan Klug wrote:\n> > When accessing a nonexistent key on a dict the YamlObject returns an\n> > empty element. This element can happily be casted to a string which is\n\ns/casted/cast/\n\n> > unexpected. For example the following statement:\n> > yamlDict[\"nonexistant\"].get(\"default\") is expected to return\n\nunexpected. For example the following statement:\n\nyamlDict[\"nonexistent\"].get(\"default\")\n\nis expected to return\n\n> s/nonexistant/nonexistent/\n> \n> > \"default\" but actually returns \"\". Fix this by introducing a empty type\n> \n> s/ a / an /\n> \n> > to distinguish between an empty YamlObject and a YamlObject of type\n> > value containing an empty string. For completeness add an isEmpty()\n> > function to be able to test for that type.\n> \n> Ooh that's a smart solution.\n\nIndeed. Normally I'd worry that this could break things in unexpected\nways, but we have unit tests :-)\n\n> > Signed-off-by: Stefan Klug \n> > ---\n> > include/libcamera/internal/yaml_parser.h | 5 +++++\n> > src/libcamera/yaml_parser.cpp | 20 ++++++++++++++------\n> > test/yaml-parser.cpp | 6 ++++++\n> > 3 files changed, 25 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > index 16708e488d88..33c3a96fed47 100644\n> > --- a/include/libcamera/internal/yaml_parser.h\n> > +++ b/include/libcamera/internal/yaml_parser.h\n> > @@ -159,6 +159,10 @@ public:\n> > \t{\n> > \t\treturn type_ == Type::Dictionary;\n> > \t}\n> > +\tbool isEmpty() const\n> > +\t{\n> > +\t\treturn type_ == Type::Empty;\n> > +\t}\n> > \n> > \tstd::size_t size() const;\n> > \n> > @@ -212,6 +216,7 @@ private:\n> > \t\tDictionary,\n> > \t\tList,\n> > \t\tValue,\n> > +\t\tEmpty,\n> > \t};\n> > \n> > \ttemplate\n> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > index 8b6a403898be..eb72529136f7 100644\n> > --- a/src/libcamera/yaml_parser.cpp\n> > +++ b/src/libcamera/yaml_parser.cpp\n> > @@ -38,12 +38,12 @@ static const YamlObject empty;\n> > * \\brief A class representing the tree structure of the YAML content\n> > *\n> > * The YamlObject class represents the tree structure of YAML content. A\n> > - * YamlObject can be a dictionary or list of YamlObjects or a value if a tree\n> > - * leaf.\n> > + * YamlObject can be empty, a dictionary or list of YamlObjects or a value if a\n\n * YamlObject can be empty, a dictionary or list of YamlObjects, or a value if a\n\n> > + * tree leaf.\n> > */\n> > \n> > YamlObject::YamlObject()\n> > -\t: type_(Type::Value)\n> > +\t: type_(Type::Empty)\n> > {\n> > }\n> > \n> > @@ -70,6 +70,13 @@ YamlObject::~YamlObject() = default;\n> > * \\return True if the YamlObject is a dictionary, false otherwise\n> > */\n> > \n> > +/**\n> > + * \\fn YamlObject::isEmpty()\n> > + * \\brief Return whether the YamlObject is a empty\n> \n> s/ a / an /\n> \n> > + *\n> > + * \\return True if the YamlObject is empty, false otherwise\n> > + */\n> > +\n> > /**\n> > * \\fn YamlObject::size()\n> > * \\brief Retrieve the number of elements in a dictionary or list YamlObject\n> > @@ -443,7 +450,8 @@ template std::optional> YamlObject::getList() const;\n> > *\n> > * This function retrieves an element of the YamlObject. Only YamlObject\n> > * instances of List type associate elements with index, calling this function\n> > - * on other types of instances is invalid and results in undefined behaviour.\n> > + * on other types of instances or with an invalid index results in an empty\n> > + * object.\n> > *\n> > * \\return The YamlObject as an element of the list\n> > */\n> > @@ -480,8 +488,8 @@ bool YamlObject::contains(const std::string &key) const\n> > *\n> > * This function retrieve 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 is invalid and results in undefined\n> > - * behaviour.\n> > + * function on other types of instances or with a non existant key results in an\n> \n> s/non existant/nonexistent/\n> \n> Reviewed-by: Paul Elder \n> \n> > + * empty object.\n> > *\n> > * \\return The YamlObject corresponding to the \\a key member\n> > */\n> > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > index 81c829834667..b8ae0b4c11f8 100644\n> > --- a/test/yaml-parser.cpp\n> > +++ b/test/yaml-parser.cpp\n> > @@ -514,6 +514,12 @@ protected:\n> > \t\t\ti++;\n> > \t\t}\n> > \n> > +\t\t/* Test access to nonexistent member */\n\ns/member/member./\n\nReviewed-by: Laurent Pinchart \n\n> > +\t\tif (dictObj[\"nonexistent\"].get(\"default\") != \"default\") {\n> > +\t\t\tcerr << \"Accessing nonexistent dict fails to return default\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > \t\t/* Make sure utils::map_keys() works on the adapter. */\n> > \t\t(void)utils::map_keys(dictObj.asDict());\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 314DDC324C\n\tfor ;\n\tTue, 17 Sep 2024 15:10:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC139634FE;\n\tTue, 17 Sep 2024 17:10:15 +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 DF0A7634F8\n\tfor ;\n\tTue, 17 Sep 2024 17:10:13 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(62-46-121-59.adsl.highway.telekom.at [62.46.121.59])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CCE6023F;\n\tTue, 17 Sep 2024 17:08:51 +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=\"arS5L85W\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726585732;\n\tbh=jC8sHKKcYdvnvurk3VE4qsXEympHGCWwaCIj5dijR0E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=arS5L85WAedRmKNp4EJkUxJ0/BPvLTPznhaZviZn1VCmsym7kr24rpm2PKjoyC1gU\n\twLihQjEHhTBT+H8CMbNu4lywt3UpUZsmksqZElIfLu6Ln9QDGXdLSZXFIzhxCnHO7I\n\tYMmN5A5jmDMgEbOAWlxWj0H54nvroLlnQ+W/l0xU=","Date":"Tue, 17 Sep 2024 18:09:38 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","Message-ID":"<20240917150938.GF17350@pendragon.ideasonboard.com>","References":"<20240916222834.464460-1-stefan.klug@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31261,"web_url":"https://patchwork.libcamera.org/comment/31261/","msgid":"","date":"2024-09-18T09:25:19","subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Barnabás,\n\nOn Tue, Sep 17, 2024 at 01:45:13PM +0000, Barnabás Pőcze wrote:\n> Hi\n> \n> \n> 2024. szeptember 17., kedd 00:28 keltezéssel, Stefan Klug írta:\n> \n> > When accessing a nonexistent key on a dict the YamlObject returns an\n> > empty element. This element can happily be casted to a string which is\n> > unexpected. For example the following statement:\n> > yamlDict[\"nonexistant\"].get(\"default\") is expected to return\n> > \"default\" but actually returns \"\". Fix this by introducing a empty type\n> > to distinguish between an empty YamlObject and a YamlObject of type\n> > value containing an empty string. For completeness add an isEmpty()\n> > function to be able to test for that type.\n> > \n> > Signed-off-by: Stefan Klug \n> > ---\n> > include/libcamera/internal/yaml_parser.h | 5 +++++\n> > src/libcamera/yaml_parser.cpp | 20 ++++++++++++++------\n> > test/yaml-parser.cpp | 6 ++++++\n> > 3 files changed, 25 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > index 16708e488d88..33c3a96fed47 100644\n> > --- a/include/libcamera/internal/yaml_parser.h\n> > +++ b/include/libcamera/internal/yaml_parser.h\n> > @@ -159,6 +159,10 @@ public:\n> > \t{\n> > \t\treturn type_ == Type::Dictionary;\n> > \t}\n> > +\tbool isEmpty() const\n> > +\t{\n> > +\t\treturn type_ == Type::Empty;\n> > +\t}\n> \n> I think `constexpr explicit operator bool() const` would be a good addition as well.\n> Then you can just do e.g.\n> \n> if (auto y = x[\"something\"]) {\n> // ...\n> }\n\nThanks for the suggestion. I'll add that. One question though. Why the\nconstexpr? I can't come up with a practical case were that can be\nresolved at compiletime.\n\nBest regards,\nStefan\n\n> \n> \n> > \n> > \tstd::size_t size() const;\n> > \n> > @@ -212,6 +216,7 @@ private:\n> > \t\tDictionary,\n> > \t\tList,\n> > \t\tValue,\n> > +\t\tEmpty,\n> > \t};\n> > [...]\n> \n> \n> Regards,\n> Barnabás Pőcze","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 53D3FC324C\n\tfor ;\n\tWed, 18 Sep 2024 09:25:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0AB09618E0;\n\tWed, 18 Sep 2024 11:25:25 +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 8CEB5618E0\n\tfor ;\n\tWed, 18 Sep 2024 11:25:22 +0200 (CEST)","from ideasonboard.com (tmo-101-1.customers.d1-online.com\n\t[80.187.101.1])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E9FDC593;\n\tWed, 18 Sep 2024 11:23:59 +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=\"VMf0f4RE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726651440;\n\tbh=fKCXvRFEnY3YqSRJp0x8mmJ/Q22ORhvdWVFnAHPRnN4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VMf0f4REHvtO8LMEuO44/LpvIXO5AV8Z2EmzGzxbuIJOkhC3ZvhgNocFwQ75JJZLr\n\tRpO4J9noRWMne5cjD5b+YT17dv/w3tSmsnzU43iEyEu2QzSLpGho1JbG2aQ1Tc7pjY\n\tkC1DiwhODalcR9hRmCwHmhAMPsF1130pIQ7pWCrg=","Date":"Wed, 18 Sep 2024 11:25:19 +0200","From":"Stefan Klug ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","Message-ID":"","References":"<20240916222834.464460-1-stefan.klug@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31267,"web_url":"https://patchwork.libcamera.org/comment/31267/","msgid":"<2pE5gggQMM9YoW3YQxdcTGgy7ZX7vVfhQKOve66vAUywES3-zp0m35dAxnGrrAEVHM9KlbomALRZhYxsBaMVjqZKqMPSqk5dJn2evF4oKfM=@protonmail.com>","date":"2024-09-18T13:07:29","subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. szeptember 18., szerda 11:25 keltezéssel, Stefan Klug írta:\n\n> Hi Barnabás,\n> \n> On Tue, Sep 17, 2024 at 01:45:13PM +0000, Barnabás Pőcze wrote:\n> > Hi\n> >\n> >\n> > 2024. szeptember 17., kedd 00:28 keltezéssel, Stefan Klug írta:\n> >\n> > > When accessing a nonexistent key on a dict the YamlObject returns an\n> > > empty element. This element can happily be casted to a string which is\n> > > unexpected. For example the following statement:\n> > > yamlDict[\"nonexistant\"].get(\"default\") is expected to return\n> > > \"default\" but actually returns \"\". Fix this by introducing a empty type\n> > > to distinguish between an empty YamlObject and a YamlObject of type\n> > > value containing an empty string. For completeness add an isEmpty()\n> > > function to be able to test for that type.\n> > >\n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > include/libcamera/internal/yaml_parser.h | 5 +++++\n> > > src/libcamera/yaml_parser.cpp | 20 ++++++++++++++------\n> > > test/yaml-parser.cpp | 6 ++++++\n> > > 3 files changed, 25 insertions(+), 6 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > > index 16708e488d88..33c3a96fed47 100644\n> > > --- a/include/libcamera/internal/yaml_parser.h\n> > > +++ b/include/libcamera/internal/yaml_parser.h\n> > > @@ -159,6 +159,10 @@ public:\n> > > \t{\n> > > \t\treturn type_ == Type::Dictionary;\n> > > \t}\n> > > +\tbool isEmpty() const\n> > > +\t{\n> > > +\t\treturn type_ == Type::Empty;\n> > > +\t}\n> >\n> > I think `constexpr explicit operator bool() const` would be a good addition as well.\n> > Then you can just do e.g.\n> >\n> > if (auto y = x[\"something\"]) {\n> > // ...\n> > }\n> \n> Thanks for the suggestion. I'll add that. One question though. Why the\n> constexpr? I can't come up with a practical case were that can be\n> resolved at compiletime.\n\nI did it mostly automatically, so I guess you're right. Habit is second nature...\n\n\nRegards,\nBarnabás Pőcze","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 E3D01C3257\n\tfor ;\n\tWed, 18 Sep 2024 13:07:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF760634FC;\n\tWed, 18 Sep 2024 15:07:36 +0200 (CEST)","from mail-40131.protonmail.ch (mail-40131.protonmail.ch\n\t[185.70.40.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CAE6C618E8\n\tfor ;\n\tWed, 18 Sep 2024 15:07:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"Q2uhaprY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1726664854; x=1726924054;\n\tbh=wGN9BtcP3q7EbvW2Zm95T71uL8Hu/Ozf9zRbZoh0Ju4=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=Q2uhaprYeOmPmllmVwU2ljqCP1LFtIgm+89M9syvVBTh8hvFWlLNu+M+3E7HabYPQ\n\t0GYlcppW7BCJhMIqjcGTNRVnHNFvPNnw6ILe31+FU5e7hOPRx28XTY/Q0H1tL1nIUb\n\tk2mLfWYqQUQItghkSTY9gm7mWPON1nhHhfIcS3at1cA7xJUEJMJqcmjI+CjiAM4UCV\n\tDu0EgAXeEEJQRxn+NDReiGW9b7tFHnDKVNpllYOQDax+eF01VP6p3YN76i/OOL/No6\n\txlabMj7SiKZ4/QntvXEHHwPNCO1x7Cnf3se8d1s4iJZ5rC+EWdF+Fvzi6YXZycjDZ+\n\tqaj8qFNiJ+CvQ==","Date":"Wed, 18 Sep 2024 13:07:29 +0000","To":"Stefan Klug ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: yaml-parser: Differentiate between empty and\n\tempty string","Message-ID":"<2pE5gggQMM9YoW3YQxdcTGgy7ZX7vVfhQKOve66vAUywES3-zp0m35dAxnGrrAEVHM9KlbomALRZhYxsBaMVjqZKqMPSqk5dJn2evF4oKfM=@protonmail.com>","In-Reply-To":"","References":"<20240916222834.464460-1-stefan.klug@ideasonboard.com>\n\t\n\t","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"0f2faf16e75ad0a22bc46c51e12870319c599a6b","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]