[v2,24/42] libcamera: value_node: Support adding nested children in one operation

26464 diff mbox series

The GlobalConfiguration class will need to add nested children to a
ValueNode. Add a new overload to the add() function for this purpose.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Documentation improvements
- Replace NULL with nullptr
- Don't move child node if add fails
---
 include/libcamera/internal/value_node.h |  3 ++
 src/libcamera/value_node.cpp            | 59 +++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

On Thu, Apr 23, 2026 at 03:59:48PM +0200, Barnabás Pőcze wrote:
> 2026. 04. 07. 17:34 keltezéssel, Laurent Pinchart írta:
> > The GlobalConfiguration class will need to add nested children to a
> > ValueNode. Add a new overload to the add() function for this purpose.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Documentation improvements
> > - Replace NULL with nullptr
> > - Don't move child node if add fails
> > ---
> >   include/libcamera/internal/value_node.h |  3 ++
> >   src/libcamera/value_node.cpp            | 59 +++++++++++++++++++++++++
> >   2 files changed, 62 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h
> > index d237b2c1e0d6..be28c2b73832 100644
> > --- a/include/libcamera/internal/value_node.h
> > +++ b/include/libcamera/internal/value_node.h
> > @@ -8,6 +8,7 @@
> >   
> >   #pragma once
> >   
> > +#include <initializer_list>
> >   #include <iterator>
> >   #include <map>
> >   #include <memory>
> > @@ -239,6 +240,8 @@ public:
> >   
> >   	ValueNode *add(std::unique_ptr<ValueNode> &&child);
> >   	ValueNode *add(std::string key, std::unique_ptr<ValueNode> &&child);
> > +	ValueNode *add(std::initializer_list<std::string_view> path,
> > +		       std::unique_ptr<ValueNode> &&child);
> >   
> >   private:
> >   	LIBCAMERA_DISABLE_COPY_AND_MOVE(ValueNode)
> > diff --git a/src/libcamera/value_node.cpp b/src/libcamera/value_node.cpp
> > index 4c3a5e4d381d..e8db7ef3c37f 100644
> > --- a/src/libcamera/value_node.cpp
> > +++ b/src/libcamera/value_node.cpp
> > @@ -13,6 +13,8 @@
> >   #include <string>
> >   #include <vector>
> >   
> > +#include <libcamera/base/log.h>
> > +#include <libcamera/base/span.h>
> >   #include <libcamera/base/utils.h>
> >   
> >   /**
> > @@ -22,6 +24,8 @@
> >   
> >   namespace libcamera {
> >   
> > +LOG_DEFINE_CATEGORY(ValueNode)
> > +
> >   namespace {
> >   
> >   /* Empty static ValueNode as a safe result for invalid operations */
> > @@ -539,4 +543,59 @@ ValueNode *ValueNode::add(std::string key, std::unique_ptr<ValueNode> &&child)
> >   	return list_.emplace_back(it->first, std::move(child)).value.get();
> >   }
> >   
> > +/**
> > + * \brief Add a child node at the given path
> > + * \param[in] path The path
> > + * \param[in] child The child node
> > + *
> > + * Add the \a child node at the given \a path starting at this node. Missing
> > + * nodes are created along the path. Nodes along the path must be empty (in
> > + * which case they are converted to the Type::Dictionary type), be a dictionary,
> > + * or be missing. Otherwise, the function returns a nullptr and the \a child is
> > + * not modified.
> > + *
> > + * Path elements are unique in the context of a parent node. If a child with the
> > + * same \a key already exist at the end of the path, the function returns a
> > + * nullptr and the \a child is not modified.
> > + *
> > + * \note Any node added along the \a path will remain even if this function
> > + * returns a failure.
> > + *
> > + * \return A pointer to the \a child node if successfully added, nullptr
> > + * otherwise
> > + */
> > +ValueNode *ValueNode::add(std::initializer_list<std::string_view> path,
> > +			  std::unique_ptr<ValueNode> &&child)
> > +{
> > +	if (!path.size())
> > +		return nullptr;
> > +
> > +	ValueNode *node = this;
> > +
> > +	for (const auto [i, name] : utils::enumerate(path)) {
> > +		auto iter = node->dictionary_.find(name);
> > +		if (iter == node->dictionary_.end()) {
> > +			std::unique_ptr<ValueNode> obj;
> > +
> > +			if (i < path.size() - 1)
> > +				obj = std::make_unique<ValueNode>();
> > +			else
> > +				obj = std::move(child);
> 
> This is not in sync with the description because it modifies
> `child` even if the later `node->add()` fails. Maybe:

Oops, good catch.

> diff --git a/src/libcamera/value_node.cpp b/src/libcamera/value_node.cpp
> index 8a1af13a5..7e021744c 100644
> --- a/src/libcamera/value_node.cpp
> +++ b/src/libcamera/value_node.cpp
> @@ -599,13 +599,10 @@ ValueNode *ValueNode::add(std::initializer_list<std::string_view> path,
>                  auto iter = node->dictionary_.find(name);
>                  if (iter == node->dictionary_.end()) {
>                          std::unique_ptr<ValueNode> obj;
> -
>                          if (i < path.size() - 1)
>                                  obj = std::make_unique<ValueNode>();
> -                       else
> -                               obj = std::move(child);
>   
> -                       node = node->add(std::string{ name }, std::move(obj));
> +                       node = node->add(std::string{ name }, obj ? std::move(obj) : std::move(child));

I'll do that.

>                          if (!node) {
>                                  Span<const std::string_view> pathName{ std::data(path), i + 1 };
>                                  LOG(ValueNode, Error)
> 
> 
> > +
> > +			node = node->add(std::string{ name }, std::move(obj));
> > +			if (!node) {
> > +				Span<const std::string_view> pathName{ std::data(path), i + 1 };
> > +				LOG(ValueNode, Error)
> > +					<< "Failed to populate '"
> > +					<< utils::join(pathName, "/") << "'";
> > +				return nullptr;
> > +			}
> > +		} else {
> > +			node = iter->second;
> > +		}
> > +	}
> > +
> > +	return node;
> > +}
> > +
> >   } /* namespace libcamera */