From patchwork Fri Sep 20 13:28:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 21292 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B59CEC3261 for ; Fri, 20 Sep 2024 13:28:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1707D63500; Fri, 20 Sep 2024 15:28:46 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eJ5foGW6"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DCEE4634F5 for ; Fri, 20 Sep 2024 15:28:42 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:8ade:938d:48b1:cede]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9FF944CE; Fri, 20 Sep 2024 15:27:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1726838838; bh=h2lcxPISNbay1MKWBI2eR/F203N3ahbcK6b0XGWIGLc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eJ5foGW6ykieKOXLJjgLTq2bF0G8r1X1UdWxDYghPg2BRZ1oxVXBqfRg6Kofx1GUx TUWSS7CLVhkR75AqMaXofzgH8eZHNTVUfDqDE8Nb47mg3QGP+mxF57x+BXLl6tZMho zHmTDg5yrFgyLzg/5/uIkxsZhsrXjqOkVwOvpYk4= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 1/3] libcamera: yaml-parser: Add additional tests Date: Fri, 20 Sep 2024 15:28:08 +0200 Message-ID: <20240920132823.88433-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240920132823.88433-1-stefan.klug@ideasonboard.com> References: <20240920132823.88433-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add additional tests in preparation for upcoming modifications on the yaml parser. These tests handle the case where the yaml file contains empty items in dictionaries or lists. E.g.: dict: key_with_value: value key_without_value: Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Reviewed-by: Paul Elder --- In the end it turned out that the changes on the YamlParser were never wrong, so these tests are somewhat superfluous. We could still merge them to test specifically for that case or drop the patch. Changes in v3: - Added seperate patch for the working tests --- test/yaml-parser.cpp | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 81c829834667..347999831d61 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -34,10 +34,12 @@ static const string testYaml = "list:\n" " - James\n" " - Mary\n" + " - \n" "dictionary:\n" " a: 1\n" " c: 3\n" " b: 2\n" + " empty:\n" "level1:\n" " level2:\n" " - [1, 2]\n" @@ -430,9 +432,10 @@ protected: if (testObjectType(listObj, "list", Type::List) != TestPass) return TestFail; - static constexpr std::array listValues{ + static constexpr std::array listValues{ "James", "Mary", + "", }; if (listObj.size() != listValues.size()) { @@ -465,16 +468,23 @@ protected: i++; } + /* Ensure that empty objects get parsed as empty strings. */ + if (!listObj[2].isValue()) { + cerr << "Empty object is not a value" << std::endl; + return TestFail; + } + /* Test dictionary object */ auto &dictObj = (*root)["dictionary"]; if (testObjectType(dictObj, "dictionary", Type::Dictionary) != TestPass) return TestFail; - static constexpr std::array, 3> dictValues{ { + static constexpr std::array, 4> dictValues{ { { "a", 1 }, { "c", 3 }, { "b", 2 }, + { "empty", -100 }, } }; size_t dictSize = dictValues.size(); @@ -505,7 +515,7 @@ protected: return TestFail; } - if (elem.get(0) != item.second) { + if (elem.get(-100) != item.second) { std::cerr << "Dictionary element " << i << " has wrong value" << std::endl; return TestFail; @@ -514,6 +524,18 @@ protected: i++; } + /* Ensure that empty objects get parsed as empty strings. */ + if (!dictObj["empty"].isValue()) { + cerr << "Empty object is not of type value" << std::endl; + return TestFail; + } + + /* Ensure that keys without values are added to a dict. */ + if (!dictObj.contains("empty")) { + cerr << "Empty element is missing in dict" << std::endl; + return TestFail; + } + /* Make sure utils::map_keys() works on the adapter. */ (void)utils::map_keys(dictObj.asDict()); From patchwork Fri Sep 20 13:28:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 21293 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 8E5E1C32D5 for ; Fri, 20 Sep 2024 13:28:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 19E2063502; Fri, 20 Sep 2024 15:28:48 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="t+7dpps9"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0CEF4634F8 for ; Fri, 20 Sep 2024 15:28:45 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:8ade:938d:48b1:cede]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2CCD2564; Fri, 20 Sep 2024 15:27:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1726838841; bh=m2RgS724KWx8eCi9Q/T7eoD8iBBuW1+7oBXMi0+RQfU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=t+7dpps9WFLFPezIP6JBf+hrMbXSIsn7YwHB2ay19sc/WFGvK9MQ72/PbK0qsb/7v xt5zNY52S6W552B4o1cqpTxIrICRlQF7aOa068v8S96T3g1s/46jpxV1D1UXLCSyWc XcuVXi0G4oXxiOYwiqE786bM4zagUjz4pelhVRaA= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 2/3] libcanera: yaml-parser: Add failing test for unexpected behavior Date: Fri, 20 Sep 2024 15:28:09 +0200 Message-ID: <20240920132823.88433-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240920132823.88433-1-stefan.klug@ideasonboard.com> References: <20240920132823.88433-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When accessing a nonexistent key on a dict the YamlObject returns an empty element. This element can happily be cast to a string. This is unexpected. For example the following statement: yamlDict["nonexistent"].get("default") is expected to return "default" but actually returns "". Add a (failing) testcase for that behavior. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Reviewed-by: Paul Elder --- Changes in v3: - Added separate patch for the failing test --- test/yaml-parser.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 347999831d61..4cc77e26ae39 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -536,6 +536,12 @@ protected: return TestFail; } + /* Test access to nonexistent member. */ + if (dictObj["nonexistent"].get("default") != "default") { + cerr << "Accessing nonexistent dict entry fails to return default" << std::endl; + return TestFail; + } + /* Make sure utils::map_keys() works on the adapter. */ (void)utils::map_keys(dictObj.asDict()); From patchwork Fri Sep 20 13:28:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 21294 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id ABE82C324C for ; Fri, 20 Sep 2024 13:28:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 36EB6634F8; Fri, 20 Sep 2024 15:28:50 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="IKcPZf3G"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D04E634F8 for ; Fri, 20 Sep 2024 15:28:47 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:8ade:938d:48b1:cede]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5306D4CE; Fri, 20 Sep 2024 15:27:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1726838843; bh=m/78O5AsKe70zi+wcmtKJzgjjeZKhZGHTSDm8j6yAqU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IKcPZf3G5gX3zni+6GARZxSfKYe6CQWJF8pFs0nI+VsR1KnDjxxyR/EvSkZQt8WLA iwgbw9RRWiKQovHJo59MK4KRMIEaeQbpcRWAfLTKp+MdW9mVrxh+kMn1qKL7Hm/6iA d6CRT3nSfXkxbQs81Wmj9NBwoHCb9mLSdEqr52Z8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 3/3] libcamera: yaml-parser: Differentiate between empty and empty string Date: Fri, 20 Sep 2024 15:28:10 +0200 Message-ID: <20240920132823.88433-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240920132823.88433-1-stefan.klug@ideasonboard.com> References: <20240920132823.88433-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When accessing a nonexistent key on a dict the YamlObject returns an empty element. This element can happily be cast to a string which is unexpected. For example the following statement: yamlDict["nonexistent"].get("default") is expected to return "default" but actually returns "". Fix this by introducing a empty type to distinguish between an empty YamlObject and a YamlObject of type value containing an empty string. For completeness add an isEmpty() function and an explicit cast to bool to be able to test for that type. Extend the tests accordingly. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Reviewed-by: Paul Elder --- Changes in v3: - Move tests for the existing functionality into own patches - Fixed some typos Changes in v2: - Added explicit to bool conversion operator - Fixed typos from review - I didn't add the rb tags, because the v2 adds quite some logic (especially tests) and this is a central piece of code --- include/libcamera/internal/yaml_parser.h | 9 ++++++++ src/libcamera/yaml_parser.cpp | 27 ++++++++++++++++++------ test/yaml-parser.cpp | 18 ++++++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h index 16708e488d88..6211ff4ae563 100644 --- a/include/libcamera/internal/yaml_parser.h +++ b/include/libcamera/internal/yaml_parser.h @@ -159,6 +159,14 @@ public: { return type_ == Type::Dictionary; } + bool isEmpty() const + { + return type_ == Type::Empty; + } + explicit operator bool() const + { + return type_ != Type::Empty; + } std::size_t size() const; @@ -212,6 +220,7 @@ private: Dictionary, List, Value, + Empty, }; template diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index 8b6a403898be..4784f2dc3d62 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -38,12 +38,12 @@ static const YamlObject empty; * \brief A class representing the tree structure of the YAML content * * The YamlObject class represents the tree structure of YAML content. A - * YamlObject can be a dictionary or list of YamlObjects or a value if a tree - * leaf. + * YamlObject can be empty, a dictionary or list of YamlObjects, or a value if a + * tree leaf. */ YamlObject::YamlObject() - : type_(Type::Value) + : type_(Type::Empty) { } @@ -70,6 +70,20 @@ YamlObject::~YamlObject() = default; * \return True if the YamlObject is a dictionary, false otherwise */ +/** + * \fn YamlObject::isEmpty() + * \brief Return whether the YamlObject is an empty + * + * \return True if the YamlObject is empty, false otherwise + */ + +/** + * \fn YamlObject::operator bool() + * \brief Return whether the YamlObject is a non-empty + * + * \return False if the YamlObject is empty, true otherwise + */ + /** * \fn YamlObject::size() * \brief Retrieve the number of elements in a dictionary or list YamlObject @@ -443,7 +457,8 @@ template std::optional> YamlObject::getList() const; * * This function retrieves an element of the YamlObject. Only YamlObject * instances of List type associate elements with index, calling this function - * on other types of instances is invalid and results in undefined behaviour. + * on other types of instances or with an invalid index results in an empty + * object. * * \return The YamlObject as an element of the list */ @@ -480,8 +495,8 @@ bool YamlObject::contains(const std::string &key) const * * This function retrieve a member of a YamlObject by name. Only YamlObject * instances of Dictionary type associate elements with names, calling this - * function on other types of instances is invalid and results in undefined - * behaviour. + * function on other types of instances or with a nonexistent key results in an + * empty object. * * \return The YamlObject corresponding to the \a key member */ diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 4cc77e26ae39..1b22c87b72f1 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -542,6 +542,24 @@ protected: return TestFail; } + /* Test nonexistent object has value type empty. */ + if (!dictObj["nonexistent"].isEmpty()) { + cerr << "Accessing nonexistent object returns non-empty object" << std::endl; + return TestFail; + } + + /* Test explicit cast to bool on an empty object returns true. */ + if (!!dictObj["empty"] != true) { + cerr << "Casting empty entry to bool returns false" << std::endl; + return TestFail; + } + + /* Test explicit cast to bool on nonexistent object returns false. */ + if (!!dictObj["nonexistent"] != false) { + cerr << "Casting nonexistent dict entry to bool returns true" << std::endl; + return TestFail; + } + /* Make sure utils::map_keys() works on the adapter. */ (void)utils::map_keys(dictObj.asDict());