Patch Detail
Show a patch.
GET /api/patches/21294/?format=api
{ "id": 21294, "url": "https://patchwork.libcamera.org/api/patches/21294/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21294/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20240920132823.88433-4-stefan.klug@ideasonboard.com>", "date": "2024-09-20T13:28:10", "name": "[v3,3/3] libcamera: yaml-parser: Differentiate between empty and empty string", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "dc409e67493500900cb921ebbd769b120a17eef0", "submitter": { "id": 184, "url": "https://patchwork.libcamera.org/api/people/184/?format=api", "name": "Stefan Klug", "email": "stefan.klug@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21294/mbox/", "series": [ { "id": 4608, "url": "https://patchwork.libcamera.org/api/series/4608/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4608", "date": "2024-09-20T13:28:07", "name": "libcamera: yaml-parser: Differentiate between empty and empty string", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4608/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21294/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21294/checks/", "tags": {}, "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 ABE82C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 20 Sep 2024 13:28:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36EB6634F8;\n\tFri, 20 Sep 2024 15:28:50 +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 3D04E634F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Sep 2024 15:28:47 +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 5306D4CE;\n\tFri, 20 Sep 2024 15:27:23 +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=\"IKcPZf3G\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726838843;\n\tbh=m/78O5AsKe70zi+wcmtKJzgjjeZKhZGHTSDm8j6yAqU=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=IKcPZf3G5gX3zni+6GARZxSfKYe6CQWJF8pFs0nI+VsR1KnDjxxyR/EvSkZQt8WLA\n\tiwgbw9RRWiKQovHJo59MK4KRMIEaeQbpcRWAfLTKp+MdW9mVrxh+kMn1qKL7Hm/6iA\n\td6CRT3nSfXkxbQs81Wmj9NBwoHCb9mLSdEqr52Z8=", "From": "Stefan Klug <stefan.klug@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Stefan Klug <stefan.klug@ideasonboard.com>", "Subject": "[PATCH v3 3/3] libcamera: yaml-parser: Differentiate between empty\n\tand 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", "Content-Transfer-Encoding": "8bit", "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>" }, "content": "When accessing a nonexistent key on a dict the YamlObject returns an\nempty element. This element can happily be cast to a string which is\nunexpected. For example the following statement:\n\nyamlDict[\"nonexistent\"].get<string>(\"default\")\n\nis expected to return \"default\" but actually returns \"\". Fix this by\nintroducing a empty type to distinguish between an empty YamlObject and\na YamlObject of type value containing an empty string. For completeness\nadd an isEmpty() function and an explicit cast to bool to be able to\ntest for that type.\n\nExtend the tests accordingly.\n\nSigned-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n\n---\nChanges in v3:\n- Move tests for the existing functionality into own patches\n- Fixed some typos\n\nChanges 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 | 18 ++++++++++++++++\n 3 files changed, 48 insertions(+), 6 deletions(-)", "diff": "diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\nindex 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<typename T>\ndiff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\nindex 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<std::vector<Size>> YamlObject::getList<Size>() 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 */\ndiff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\nindex 4cc77e26ae39..1b22c87b72f1 100644\n--- a/test/yaml-parser.cpp\n+++ b/test/yaml-parser.cpp\n@@ -542,6 +542,24 @@ protected:\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 explicit cast to bool on an empty object returns 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 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 \t\t/* Make sure utils::map_keys() works on the adapter. */\n \t\t(void)utils::map_keys(dictObj.asDict());\n \n", "prefixes": [ "v3", "3/3" ] }