[{"id":38635,"web_url":"https://patchwork.libcamera.org/comment/38635/","msgid":"<20260423212251.GD3132278@killaraus.ideasonboard.com>","date":"2026-04-23T21:22:51","subject":"Re: [PATCH v2 24/42] libcamera: value_node: Support adding nested\n\tchildren in one operation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Apr 23, 2026 at 03:59:48PM +0200, Barnabás Pőcze wrote:\n> 2026. 04. 07. 17:34 keltezéssel, Laurent Pinchart írta:\n> > The GlobalConfiguration class will need to add nested children to a\n> > ValueNode. Add a new overload to the add() function for this purpose.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes since v1:\n> > \n> > - Documentation improvements\n> > - Replace NULL with nullptr\n> > - Don't move child node if add fails\n> > ---\n> > include/libcamera/internal/value_node.h | 3 ++\n> > src/libcamera/value_node.cpp | 59 +++++++++++++++++++++++++\n> > 2 files changed, 62 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h\n> > index d237b2c1e0d6..be28c2b73832 100644\n> > --- a/include/libcamera/internal/value_node.h\n> > +++ b/include/libcamera/internal/value_node.h\n> > @@ -8,6 +8,7 @@\n> > \n> > #pragma once\n> > \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -239,6 +240,8 @@ public:\n> > \n> > \tValueNode *add(std::unique_ptr &&child);\n> > \tValueNode *add(std::string key, std::unique_ptr &&child);\n> > +\tValueNode *add(std::initializer_list path,\n> > +\t\t std::unique_ptr &&child);\n> > \n> > private:\n> > \tLIBCAMERA_DISABLE_COPY_AND_MOVE(ValueNode)\n> > diff --git a/src/libcamera/value_node.cpp b/src/libcamera/value_node.cpp\n> > index 4c3a5e4d381d..e8db7ef3c37f 100644\n> > --- a/src/libcamera/value_node.cpp\n> > +++ b/src/libcamera/value_node.cpp\n> > @@ -13,6 +13,8 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > +#include \n> > #include \n> > \n> > /**\n> > @@ -22,6 +24,8 @@\n> > \n> > namespace libcamera {\n> > \n> > +LOG_DEFINE_CATEGORY(ValueNode)\n> > +\n> > namespace {\n> > \n> > /* Empty static ValueNode as a safe result for invalid operations */\n> > @@ -539,4 +543,59 @@ ValueNode *ValueNode::add(std::string key, std::unique_ptr &&child)\n> > \treturn list_.emplace_back(it->first, std::move(child)).value.get();\n> > }\n> > \n> > +/**\n> > + * \\brief Add a child node at the given path\n> > + * \\param[in] path The path\n> > + * \\param[in] child The child node\n> > + *\n> > + * Add the \\a child node at the given \\a path starting at this node. Missing\n> > + * nodes are created along the path. Nodes along the path must be empty (in\n> > + * which case they are converted to the Type::Dictionary type), be a dictionary,\n> > + * or be missing. Otherwise, the function returns a nullptr and the \\a child is\n> > + * not modified.\n> > + *\n> > + * Path elements are unique in the context of a parent node. If a child with the\n> > + * same \\a key already exist at the end of the path, the function returns a\n> > + * nullptr and the \\a child is not modified.\n> > + *\n> > + * \\note Any node added along the \\a path will remain even if this function\n> > + * returns a failure.\n> > + *\n> > + * \\return A pointer to the \\a child node if successfully added, nullptr\n> > + * otherwise\n> > + */\n> > +ValueNode *ValueNode::add(std::initializer_list path,\n> > +\t\t\t std::unique_ptr &&child)\n> > +{\n> > +\tif (!path.size())\n> > +\t\treturn nullptr;\n> > +\n> > +\tValueNode *node = this;\n> > +\n> > +\tfor (const auto [i, name] : utils::enumerate(path)) {\n> > +\t\tauto iter = node->dictionary_.find(name);\n> > +\t\tif (iter == node->dictionary_.end()) {\n> > +\t\t\tstd::unique_ptr obj;\n> > +\n> > +\t\t\tif (i < path.size() - 1)\n> > +\t\t\t\tobj = std::make_unique();\n> > +\t\t\telse\n> > +\t\t\t\tobj = std::move(child);\n> \n> This is not in sync with the description because it modifies\n> `child` even if the later `node->add()` fails. Maybe:\n\nOops, good catch.\n\n> diff --git a/src/libcamera/value_node.cpp b/src/libcamera/value_node.cpp\n> index 8a1af13a5..7e021744c 100644\n> --- a/src/libcamera/value_node.cpp\n> +++ b/src/libcamera/value_node.cpp\n> @@ -599,13 +599,10 @@ ValueNode *ValueNode::add(std::initializer_list path,\n> auto iter = node->dictionary_.find(name);\n> if (iter == node->dictionary_.end()) {\n> std::unique_ptr obj;\n> -\n> if (i < path.size() - 1)\n> obj = std::make_unique();\n> - else\n> - obj = std::move(child);\n> \n> - node = node->add(std::string{ name }, std::move(obj));\n> + node = node->add(std::string{ name }, obj ? std::move(obj) : std::move(child));\n\nI'll do that.\n\n> if (!node) {\n> Span pathName{ std::data(path), i + 1 };\n> LOG(ValueNode, Error)\n> \n> \n> > +\n> > +\t\t\tnode = node->add(std::string{ name }, std::move(obj));\n> > +\t\t\tif (!node) {\n> > +\t\t\t\tSpan pathName{ std::data(path), i + 1 };\n> > +\t\t\t\tLOG(ValueNode, Error)\n> > +\t\t\t\t\t<< \"Failed to populate '\"\n> > +\t\t\t\t\t<< utils::join(pathName, \"/\") << \"'\";\n> > +\t\t\t\treturn nullptr;\n> > +\t\t\t}\n> > +\t\t} else {\n> > +\t\t\tnode = iter->second;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\treturn node;\n> > +}\n> > +\n> > } /* namespace libcamera */","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 47FD0BDCB5\n\tfor ;\n\tThu, 23 Apr 2026 21:22:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 766F262F5D;\n\tThu, 23 Apr 2026 23:22:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B0DC62010\n\tfor ;\n\tThu, 23 Apr 2026 23:22:53 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-703d-e500--2a1.rev.dnainternet.fi\n\t[IPv6:2001:14ba:703d:e500::2a1])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EEB149C;\n\tThu, 23 Apr 2026 23:21:12 +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=\"jidpXSiO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1776979273;\n\tbh=DUJ/9tLJwl1s0tKHDO9eYmtKSgOUsutPkiZ+WxdpMEs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jidpXSiOruiTbNcAXuf0C9CKpIDOj86iWESFIFvLqtGcXf2uPFXHvl4LBjcLxAfRp\n\tUz1uKXeoK7NO27QWnBGxQwPmimLk4DfybI6SjFQhe+3JTe+9ddP2P478OR2EhgxL1u\n\tpxqOeES2Q9l9Dh5ZfVye1rX4USKLDty7A871DjO8=","Date":"Fri, 24 Apr 2026 00:22:51 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 24/42] libcamera: value_node: Support adding nested\n\tchildren in one operation","Message-ID":"<20260423212251.GD3132278@killaraus.ideasonboard.com>","References":"<20260407153427.1825999-1-laurent.pinchart@ideasonboard.com>\n\t<20260407153427.1825999-25-laurent.pinchart@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\" "}}]