[{"id":31282,"web_url":"https://patchwork.libcamera.org/comment/31282/","msgid":"<20240919222408.GC29185@pendragon.ideasonboard.com>","date":"2024-09-19T22:24:08","subject":"Re: [PATCH v2] libcamera: yaml-parser: Differentiate between empty\n\tand empty string","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nThank you for the patch.\n\nOn Thu, Sep 19, 2024 at 04:17:41PM +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> \n> yamlDict[\"nonexistant\"].get(\"default\")\n> \n> is expected to return \"default\" but actually returns \"\". Fix this by\n> introducing a empty type to distinguish between an empty YamlObject and\n> a YamlObject of type value containing an empty string. For completeness\n> add an isEmpty() function and an explicit cast to bool to be able to\n> test for that type.\n> \n> The tests are adapted accordingly. Additional tests for empty entries in\n> lists and dicts are added to ensure that these still get parsed as empty\n> strings. This is needed for algorithms without parameters to be parsed\n> correctly from tuning files.\n> \n> Signed-off-by: Stefan Klug \n> \n> ---\n> Changes in v2:\n> - Added explicit to bool conversion operator\n> - Fixed typos from review\n> - I didn't add the rb tags, because the v2 adds quite some logic\n> (especially tests) and this is a central piece of code\n> ---\n> include/libcamera/internal/yaml_parser.h | 9 +++++\n> src/libcamera/yaml_parser.cpp | 27 ++++++++++---\n> test/yaml-parser.cpp | 51 ++++++++++++++++++++++--\n> 3 files changed, 78 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> index 16708e488d88..6211ff4ae563 100644\n> --- a/include/libcamera/internal/yaml_parser.h\n> +++ b/include/libcamera/internal/yaml_parser.h\n> @@ -159,6 +159,14 @@ 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> +\texplicit operator bool() const\n> +\t{\n> +\t\treturn type_ != Type::Empty;\n> +\t}\n> \n> \tstd::size_t size() const;\n> \n> @@ -212,6 +220,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..4784f2dc3d62 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,20 @@ 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 an empty\n> + *\n> + * \\return True if the YamlObject is empty, false otherwise\n> + */\n> +\n> +/**\n> + * \\fn YamlObject::operator bool()\n> + * \\brief Return whether the YamlObject is a non-empty\n> + *\n> + * \\return False if the YamlObject is empty, true otherwise\n> + */\n> +\n> /**\n> * \\fn YamlObject::size()\n> * \\brief Retrieve the number of elements in a dictionary or list YamlObject\n> @@ -443,7 +457,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 +495,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 nonexistent key results in an\n> + * empty object.\n> *\n> * \\return The YamlObject corresponding to the \\a key member\n> */\n\nThis part looks good to me.\n\n> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> index 81c829834667..9d340e29d4a9 100644\n> --- a/test/yaml-parser.cpp\n> +++ b/test/yaml-parser.cpp\n\nCould you split this in a separate patch that goes *first* in the series\n? The newly introduced tests should fail, and then the second patch\nshould fix them. This helps ensuring that your tests actually test what\nyou think they test.\n\n> @@ -34,10 +34,12 @@ static const string testYaml =\n> \t\"list:\\n\"\n> \t\" - James\\n\"\n> \t\" - Mary\\n\"\n> +\t\" - \\n\"\n> \t\"dictionary:\\n\"\n> \t\" a: 1\\n\"\n> \t\" c: 3\\n\"\n> \t\" b: 2\\n\"\n> +\t\" empty:\\n\"\n> \t\"level1:\\n\"\n> \t\" level2:\\n\"\n> \t\" - [1, 2]\\n\"\n> @@ -430,9 +432,10 @@ protected:\n> \t\tif (testObjectType(listObj, \"list\", Type::List) != TestPass)\n> \t\t\treturn TestFail;\n> \n> -\t\tstatic constexpr std::array listValues{\n> +\t\tstatic constexpr std::array listValues{\n> \t\t\t\"James\",\n> \t\t\t\"Mary\",\n> +\t\t\t\"\",\n> \t\t};\n> \n> \t\tif (listObj.size() != listValues.size()) {\n> @@ -465,16 +468,22 @@ protected:\n> \t\t\ti++;\n> \t\t}\n> \n> +\t\t/* Ensure that empty objects get parsed as empty strings. */\n> +\t\tif (!listObj[2].isValue()) {\n> +\t\t\tcerr << \"Empty object is not a value\" << std::endl;\n\nShouldn't this return TestFail ?\n\n> +\t\t}\n> +\n> \t\t/* Test dictionary object */\n> \t\tauto &dictObj = (*root)[\"dictionary\"];\n> \n> \t\tif (testObjectType(dictObj, \"dictionary\", Type::Dictionary) != TestPass)\n> \t\t\treturn TestFail;\n> \n> -\t\tstatic constexpr std::array, 3> dictValues{ {\n> +\t\tstatic constexpr std::array, 4> dictValues{ {\n> \t\t\t{ \"a\", 1 },\n> \t\t\t{ \"c\", 3 },\n> \t\t\t{ \"b\", 2 },\n> +\t\t\t{ \"empty\", -100 },\n> \t\t} };\n> \n> \t\tsize_t dictSize = dictValues.size();\n> @@ -505,7 +514,7 @@ protected:\n> \t\t\t\treturn TestFail;\n> \t\t\t}\n> \n> -\t\t\tif (elem.get(0) != item.second) {\n> +\t\t\tif (elem.get(-100) != item.second) {\n\nIs this needed, can't you keep 0 as the default value (and update\ndictValues) accordingly ?\n\n> \t\t\t\tstd::cerr << \"Dictionary element \" << i << \" has wrong value\"\n> \t\t\t\t\t << std::endl;\n> \t\t\t\treturn TestFail;\n> @@ -514,6 +523,42 @@ protected:\n> \t\t\ti++;\n> \t\t}\n> \n> +\t\t/* Ensure that empty objects get parsed as empty strings. */\n> +\t\tif (!dictObj[\"empty\"].isValue()) {\n> +\t\t\tcerr << \"Empty object is not of type value\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Ensure that keys without values are still added to a dict. */\n\ns/still //\n\n> +\t\tif (!dictObj.contains(\"empty\")) {\n> +\t\t\tcerr << \"Empty element is missing in dict\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test explicit cast to bool on an empty object must return true. */\n> +\t\tif (!!dictObj[\"empty\"] != true) {\n> +\t\t\tcerr << \"Casting empty entry to bool returns false\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test nonexistent object has value type empty. */\n> +\t\tif (!dictObj[\"nonexistent\"].isEmpty()) {\n> +\t\t\tcerr << \"Accessing nonexistent object returns non-empty object\" << std::endl;\n> +\t\t\treturn TestFail;\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 entry fails to return default\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Test explicit cast to bool on nonexistent object returns false. */\n> +\t\tif (!!dictObj[\"nonexistent\"] != false) {\n> +\t\t\tcerr << \"Casting nonexistent dict entry to bool returns true\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n\nI would split this new test in two, with one patch for all the tests\nthat currently succeed, and one for the test that fails.\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 DCF32C3257\n\tfor ;\n\tThu, 19 Sep 2024 22:24:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E4A2634FC;\n\tFri, 20 Sep 2024 00:24:43 +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 49EBA618E8\n\tfor ;\n\tFri, 20 Sep 2024 00:24:41 +0200 (CEST)","from pendragon.ideasonboard.com (213142096159.public.telering.at\n\t[213.142.96.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6B34E226;\n\tFri, 20 Sep 2024 00:23:17 +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=\"VjBYqpQB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726784597;\n\tbh=xAH/nPGg4FMryYdeI3c8vteBtXPvYs8cisBfv+utFTU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VjBYqpQBzmow/SMKOqU4h9BX54sEnQPMwwDkKTck/t9iGglea47K96ESoaGm1/1SN\n\trYSIjG4DfKUGDAzFTgcHLS/nVls5WjNlymScJZkLItd3dJZmlul2Y4pfXhzXzjpCWH\n\t8u/weEo7dG9tZe5AKyA1hM027Myy9j4UmT10hZ4A=","Date":"Fri, 20 Sep 2024 01:24:08 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: yaml-parser: Differentiate between empty\n\tand empty string","Message-ID":"<20240919222408.GC29185@pendragon.ideasonboard.com>","References":"<20240919141832.40249-1-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240919141832.40249-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":31283,"web_url":"https://patchwork.libcamera.org/comment/31283/","msgid":"","date":"2024-09-20T08:41:03","subject":"Re: [PATCH v2] libcamera: yaml-parser: Differentiate between empty\n\tand empty string","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the review. \n\nOn Fri, Sep 20, 2024 at 01:24:08AM +0300, Laurent Pinchart wrote:\n> Hi Stefan,\n> \n> Thank you for the patch.\n> \n> On Thu, Sep 19, 2024 at 04:17:41PM +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> > \n> > yamlDict[\"nonexistant\"].get(\"default\")\n> > \n> > is expected to return \"default\" but actually returns \"\". Fix this by\n> > introducing a empty type to distinguish between an empty YamlObject and\n> > a YamlObject of type value containing an empty string. For completeness\n> > add an isEmpty() function and an explicit cast to bool to be able to\n> > test for that type.\n> > \n> > The tests are adapted accordingly. Additional tests for empty entries in\n> > lists and dicts are added to ensure that these still get parsed as empty\n> > strings. This is needed for algorithms without parameters to be parsed\n> > correctly from tuning files.\n> > \n> > Signed-off-by: Stefan Klug \n> > \n> > ---\n> > Changes in v2:\n> > - Added explicit to bool conversion operator\n> > - Fixed typos from review\n> > - I didn't add the rb tags, because the v2 adds quite some logic\n> > (especially tests) and this is a central piece of code\n> > ---\n> > include/libcamera/internal/yaml_parser.h | 9 +++++\n> > src/libcamera/yaml_parser.cpp | 27 ++++++++++---\n> > test/yaml-parser.cpp | 51 ++++++++++++++++++++++--\n> > 3 files changed, 78 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > index 16708e488d88..6211ff4ae563 100644\n> > --- a/include/libcamera/internal/yaml_parser.h\n> > +++ b/include/libcamera/internal/yaml_parser.h\n> > @@ -159,6 +159,14 @@ 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> > +\texplicit operator bool() const\n> > +\t{\n> > +\t\treturn type_ != Type::Empty;\n> > +\t}\n> > \n> > \tstd::size_t size() const;\n> > \n> > @@ -212,6 +220,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..4784f2dc3d62 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,20 @@ 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 an empty\n> > + *\n> > + * \\return True if the YamlObject is empty, false otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\fn YamlObject::operator bool()\n> > + * \\brief Return whether the YamlObject is a non-empty\n> > + *\n> > + * \\return False if the YamlObject is empty, true otherwise\n> > + */\n> > +\n> > /**\n> > * \\fn YamlObject::size()\n> > * \\brief Retrieve the number of elements in a dictionary or list YamlObject\n> > @@ -443,7 +457,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 +495,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 nonexistent key results in an\n> > + * empty object.\n> > *\n> > * \\return The YamlObject corresponding to the \\a key member\n> > */\n> \n> This part looks good to me.\n> \n> > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > index 81c829834667..9d340e29d4a9 100644\n> > --- a/test/yaml-parser.cpp\n> > +++ b/test/yaml-parser.cpp\n> \n> Could you split this in a separate patch that goes *first* in the series\n> ? The newly introduced tests should fail, and then the second patch\n> should fix them. This helps ensuring that your tests actually test what\n> you think they test.\n\nI thought about that before. Yes that would have made it clearer. I'll\nchanges that.\n\n> \n> > @@ -34,10 +34,12 @@ static const string testYaml =\n> > \t\"list:\\n\"\n> > \t\" - James\\n\"\n> > \t\" - Mary\\n\"\n> > +\t\" - \\n\"\n> > \t\"dictionary:\\n\"\n> > \t\" a: 1\\n\"\n> > \t\" c: 3\\n\"\n> > \t\" b: 2\\n\"\n> > +\t\" empty:\\n\"\n> > \t\"level1:\\n\"\n> > \t\" level2:\\n\"\n> > \t\" - [1, 2]\\n\"\n> > @@ -430,9 +432,10 @@ protected:\n> > \t\tif (testObjectType(listObj, \"list\", Type::List) != TestPass)\n> > \t\t\treturn TestFail;\n> > \n> > -\t\tstatic constexpr std::array listValues{\n> > +\t\tstatic constexpr std::array listValues{\n> > \t\t\t\"James\",\n> > \t\t\t\"Mary\",\n> > +\t\t\t\"\",\n> > \t\t};\n> > \n> > \t\tif (listObj.size() != listValues.size()) {\n> > @@ -465,16 +468,22 @@ protected:\n> > \t\t\ti++;\n> > \t\t}\n> > \n> > +\t\t/* Ensure that empty objects get parsed as empty strings. */\n> > +\t\tif (!listObj[2].isValue()) {\n> > +\t\t\tcerr << \"Empty object is not a value\" << std::endl;\n> \n> Shouldn't this return TestFail ?\n\nOh missed that. Thanks.\n\n> \n> > +\t\t}\n> > +\n> > \t\t/* Test dictionary object */\n> > \t\tauto &dictObj = (*root)[\"dictionary\"];\n> > \n> > \t\tif (testObjectType(dictObj, \"dictionary\", Type::Dictionary) != TestPass)\n> > \t\t\treturn TestFail;\n> > \n> > -\t\tstatic constexpr std::array, 3> dictValues{ {\n> > +\t\tstatic constexpr std::array, 4> dictValues{ {\n> > \t\t\t{ \"a\", 1 },\n> > \t\t\t{ \"c\", 3 },\n> > \t\t\t{ \"b\", 2 },\n> > +\t\t\t{ \"empty\", -100 },\n> > \t\t} };\n> > \n> > \t\tsize_t dictSize = dictValues.size();\n> > @@ -505,7 +514,7 @@ protected:\n> > \t\t\t\treturn TestFail;\n> > \t\t\t}\n> > \n> > -\t\t\tif (elem.get(0) != item.second) {\n> > +\t\t\tif (elem.get(-100) != item.second) {\n> \n> Is this needed, can't you keep 0 as the default value (and update\n> dictValues) accordingly ?\n\nThe idea was to have a explicit sentinel value, that will never occur by\na failed string to int conversion or something returning 0 as fallback.\nAnd having 0 in dictValues seems not right. The -100 is equally wrong\nbut it seems clearer to the reader that that was on purpose. I can\nchange that back to 0 if you like - up to you. \n\n> \n> > \t\t\t\tstd::cerr << \"Dictionary element \" << i << \" has wrong value\"\n> > \t\t\t\t\t << std::endl;\n> > \t\t\t\treturn TestFail;\n> > @@ -514,6 +523,42 @@ protected:\n> > \t\t\ti++;\n> > \t\t}\n> > \n> > +\t\t/* Ensure that empty objects get parsed as empty strings. */\n> > +\t\tif (!dictObj[\"empty\"].isValue()) {\n> > +\t\t\tcerr << \"Empty object is not of type value\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Ensure that keys without values are still added to a dict. */\n> \n> s/still //\n> \n> > +\t\tif (!dictObj.contains(\"empty\")) {\n> > +\t\t\tcerr << \"Empty element is missing in dict\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test explicit cast to bool on an empty object must return true. */\n> > +\t\tif (!!dictObj[\"empty\"] != true) {\n> > +\t\t\tcerr << \"Casting empty entry to bool returns false\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test nonexistent object has value type empty. */\n> > +\t\tif (!dictObj[\"nonexistent\"].isEmpty()) {\n> > +\t\t\tcerr << \"Accessing nonexistent object returns non-empty object\" << std::endl;\n> > +\t\t\treturn TestFail;\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 entry fails to return default\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Test explicit cast to bool on nonexistent object returns false. */\n> > +\t\tif (!!dictObj[\"nonexistent\"] != false) {\n> > +\t\t\tcerr << \"Casting nonexistent dict entry to bool returns true\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> \n> I would split this new test in two, with one patch for all the tests\n> that currently succeed, and one for the test that fails.\n\nYes, I'll do that.\n\nRegards,\nStefan\n\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> -- \n> Regards,\n> \n> Laurent Pinchart","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 3A2B9C324C\n\tfor ;\n\tFri, 20 Sep 2024 08:41:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC15F634FC;\n\tFri, 20 Sep 2024 10:41:07 +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 29278618E3\n\tfor ;\n\tFri, 20 Sep 2024 10:41:06 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:8ade:938d:48b1:cede])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 28C5D4CE;\n\tFri, 20 Sep 2024 10:39:42 +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=\"CiDUaA7l\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726821582;\n\tbh=qJCum1zr+yWscbGItFQIRvVQRT+D1HRtjY7XHa45XWQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CiDUaA7lUXOmHpJhq1652JmoONvs2EtUMlMUDgRBQgN/CdEcVp3qTSXQ+keIYeoLM\n\t94zTK5FSAfLyFtYsLEMO9rcT8xZ3DOwpIxH+stctY6wsKYiq/4HY7CJvzOa2cdvzKD\n\ttCUKYavG3YXVIFuFNF7lkrkKirtxnGA8Oc35GDGw=","Date":"Fri, 20 Sep 2024 10:41:03 +0200","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: yaml-parser: Differentiate between empty\n\tand empty string","Message-ID":"","References":"<20240919141832.40249-1-stefan.klug@ideasonboard.com>\n\t<20240919222408.GC29185@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240919222408.GC29185@pendragon.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\" "}}]