| Message ID | 20260113000808.15395-14-laurent.pinchart@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: > Add YamlObject::add() functions to add children to a list or dictionary > object. This will be used by the YamlParserContext to replace direct > access to YamlObject member variables to decouple the two classes. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/yaml_parser.h | 3 ++ > src/libcamera/yaml_parser.cpp | 56 ++++++++++++++++++++++++ > 2 files changed, 59 insertions(+) > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h > index c98fe003c877..ede2c3de687c 100644 > --- a/include/libcamera/internal/yaml_parser.h > +++ b/include/libcamera/internal/yaml_parser.h > @@ -196,6 +196,9 @@ public: > bool contains(std::string_view key) const; > const YamlObject &operator[](std::string_view key) const; > > + YamlObject *add(std::unique_ptr<YamlObject> child); > + YamlObject *add(std::string key, std::unique_ptr<YamlObject> child); > + > private: > LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject) > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp > index 9b61d3e8fc50..7462e2d74179 100644 > --- a/src/libcamera/yaml_parser.cpp > +++ b/src/libcamera/yaml_parser.cpp > @@ -448,6 +448,62 @@ const YamlObject &YamlObject::operator[](std::string_view key) const > return *iter->second; > } > > +/** > + * \brief Add a child object to a list > + * \param[in] child The child object > + * > + * Append the \a child node as the last element of this node's children list. > + * This node must be empty, in which case it is converted to the Type::List > + * type, or be a list. Otherwise, the \a child is discarded and the function > + * returns a nullptr. > + * > + * \return A pointer to the child object if successfully added, nullptr > + * otherwise > + */ > +YamlObject *YamlObject::add(std::unique_ptr<YamlObject> child) > +{ > + if (type_ == Type::Empty) > + type_ = Type::List; > + > + if (type_ != Type::List) > + return nullptr; > + > + Value &elem = list_.emplace_back(std::string{}, std::move(child)); > + return elem.value.get(); > +} > + > +/** > + * \brief Add a child object to a dictionary > + * \param[in] key The dictionary key > + * \param[in] child The child object > + * > + * Add the \a child node with the given \a key to this node's children. This > + * node must be empty, in which case it is converted to the Type::Dictionary > + * type, or be a dictionary. Otherwise, the \a child is discarded and the > + * function returns a nullptr. > + * > + * Keys are unique. If a child with the same \a key already exist, the \a child > + * is discarded and the function returns a nullptr. I'm wondering if other behaviours, such as overwriting the child object make sense. Any thoughts? Or are these very specifically here to work with YamlParserContext? > + * > + * \return A pointer to the child object if successfully added, nullptr > + * otherwise > + */ > +YamlObject *YamlObject::add(std::string key, std::unique_ptr<YamlObject> child) > +{ > + if (type_ == Type::Empty) > + type_ = Type::Dictionary; > + > + if (type_ != Type::Dictionary) > + return nullptr; > + > + if (dictionary_.find(key) != dictionary_.end()) > + return nullptr; > + > + Value &elem = list_.emplace_back(std::move(key), std::move(child)); > + dictionary_.emplace(elem.key, elem.value.get()); > + return elem.value.get(); Since `key` is already a string: auto [it, inserted] = dictionary_.try_emplace(std::move(key), child.get()); if (!inserted) return nullptr; return list_.emplace_back(it->first, std::move(child)).value.get(); > +} > + > #ifndef __DOXYGEN__ > > class YamlParserContext > -- > Regards, > > Laurent Pinchart >
On Tue, Jan 13, 2026 at 10:43:18AM +0100, Barnabás Pőcze wrote: > 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: > > Add YamlObject::add() functions to add children to a list or dictionary > > object. This will be used by the YamlParserContext to replace direct > > access to YamlObject member variables to decouple the two classes. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/internal/yaml_parser.h | 3 ++ > > src/libcamera/yaml_parser.cpp | 56 ++++++++++++++++++++++++ > > 2 files changed, 59 insertions(+) > > > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h > > index c98fe003c877..ede2c3de687c 100644 > > --- a/include/libcamera/internal/yaml_parser.h > > +++ b/include/libcamera/internal/yaml_parser.h > > @@ -196,6 +196,9 @@ public: > > bool contains(std::string_view key) const; > > const YamlObject &operator[](std::string_view key) const; > > > > + YamlObject *add(std::unique_ptr<YamlObject> child); > > + YamlObject *add(std::string key, std::unique_ptr<YamlObject> child); > > + > > private: > > LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject) > > > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp > > index 9b61d3e8fc50..7462e2d74179 100644 > > --- a/src/libcamera/yaml_parser.cpp > > +++ b/src/libcamera/yaml_parser.cpp > > @@ -448,6 +448,62 @@ const YamlObject &YamlObject::operator[](std::string_view key) const > > return *iter->second; > > } > > > > +/** > > + * \brief Add a child object to a list > > + * \param[in] child The child object > > + * > > + * Append the \a child node as the last element of this node's children list. > > + * This node must be empty, in which case it is converted to the Type::List > > + * type, or be a list. Otherwise, the \a child is discarded and the function > > + * returns a nullptr. > > + * > > + * \return A pointer to the child object if successfully added, nullptr > > + * otherwise > > + */ > > +YamlObject *YamlObject::add(std::unique_ptr<YamlObject> child) > > +{ > > + if (type_ == Type::Empty) > > + type_ = Type::List; > > + > > + if (type_ != Type::List) > > + return nullptr; > > + > > + Value &elem = list_.emplace_back(std::string{}, std::move(child)); > > + return elem.value.get(); > > +} > > + > > +/** > > + * \brief Add a child object to a dictionary > > + * \param[in] key The dictionary key > > + * \param[in] child The child object > > + * > > + * Add the \a child node with the given \a key to this node's children. This > > + * node must be empty, in which case it is converted to the Type::Dictionary > > + * type, or be a dictionary. Otherwise, the \a child is discarded and the > > + * function returns a nullptr. > > + * > > + * Keys are unique. If a child with the same \a key already exist, the \a child > > + * is discarded and the function returns a nullptr. > > I'm wondering if other behaviours, such as overwriting the child object make sense. > Any thoughts? Or are these very specifically here to work with YamlParserContext? There should be no duplicate keys in YAML so both behaviours should work fine for this use case. I don't know at this point which behaviour would be the most sensible for the majority of use cases. > > + * > > + * \return A pointer to the child object if successfully added, nullptr > > + * otherwise > > + */ > > +YamlObject *YamlObject::add(std::string key, std::unique_ptr<YamlObject> child) > > +{ > > + if (type_ == Type::Empty) > > + type_ = Type::Dictionary; > > + > > + if (type_ != Type::Dictionary) > > + return nullptr; > > + > > + if (dictionary_.find(key) != dictionary_.end()) > > + return nullptr; > > + > > + Value &elem = list_.emplace_back(std::move(key), std::move(child)); > > + dictionary_.emplace(elem.key, elem.value.get()); > > + return elem.value.get(); > > Since `key` is already a string: > > auto [it, inserted] = dictionary_.try_emplace(std::move(key), child.get()); > if (!inserted) > return nullptr; > > return list_.emplace_back(it->first, std::move(child)).value.get(); I assume you meant that this would replace all code from dictionary_.find() onwards. It seems fine, I'll do that in v2. > > +} > > + > > #ifndef __DOXYGEN__ > > > > class YamlParserContext
On Tue, Jan 13, 2026 at 04:03:55PM +0200, Laurent Pinchart wrote: > On Tue, Jan 13, 2026 at 10:43:18AM +0100, Barnabás Pőcze wrote: > > 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: > > > Add YamlObject::add() functions to add children to a list or dictionary > > > object. This will be used by the YamlParserContext to replace direct > > > access to YamlObject member variables to decouple the two classes. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > include/libcamera/internal/yaml_parser.h | 3 ++ > > > src/libcamera/yaml_parser.cpp | 56 ++++++++++++++++++++++++ > > > 2 files changed, 59 insertions(+) > > > > > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h > > > index c98fe003c877..ede2c3de687c 100644 > > > --- a/include/libcamera/internal/yaml_parser.h > > > +++ b/include/libcamera/internal/yaml_parser.h > > > @@ -196,6 +196,9 @@ public: > > > bool contains(std::string_view key) const; > > > const YamlObject &operator[](std::string_view key) const; > > > > > > + YamlObject *add(std::unique_ptr<YamlObject> child); > > > + YamlObject *add(std::string key, std::unique_ptr<YamlObject> child); > > > + > > > private: > > > LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject) > > > > > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp > > > index 9b61d3e8fc50..7462e2d74179 100644 > > > --- a/src/libcamera/yaml_parser.cpp > > > +++ b/src/libcamera/yaml_parser.cpp > > > @@ -448,6 +448,62 @@ const YamlObject &YamlObject::operator[](std::string_view key) const > > > return *iter->second; > > > } > > > > > > +/** > > > + * \brief Add a child object to a list > > > + * \param[in] child The child object > > > + * > > > + * Append the \a child node as the last element of this node's children list. > > > + * This node must be empty, in which case it is converted to the Type::List > > > + * type, or be a list. Otherwise, the \a child is discarded and the function > > > + * returns a nullptr. > > > + * > > > + * \return A pointer to the child object if successfully added, nullptr > > > + * otherwise > > > + */ > > > +YamlObject *YamlObject::add(std::unique_ptr<YamlObject> child) > > > +{ > > > + if (type_ == Type::Empty) > > > + type_ = Type::List; > > > + > > > + if (type_ != Type::List) > > > + return nullptr; > > > + > > > + Value &elem = list_.emplace_back(std::string{}, std::move(child)); > > > + return elem.value.get(); > > > +} > > > + > > > +/** > > > + * \brief Add a child object to a dictionary > > > + * \param[in] key The dictionary key > > > + * \param[in] child The child object > > > + * > > > + * Add the \a child node with the given \a key to this node's children. This > > > + * node must be empty, in which case it is converted to the Type::Dictionary > > > + * type, or be a dictionary. Otherwise, the \a child is discarded and the > > > + * function returns a nullptr. > > > + * > > > + * Keys are unique. If a child with the same \a key already exist, the \a child > > > + * is discarded and the function returns a nullptr. > > > > I'm wondering if other behaviours, such as overwriting the child object make sense. > > Any thoughts? Or are these very specifically here to work with YamlParserContext? > > There should be no duplicate keys in YAML so both behaviours should work > fine for this use case. I don't know at this point which behaviour would > be the most sensible for the majority of use cases. > > > > + * > > > + * \return A pointer to the child object if successfully added, nullptr > > > + * otherwise > > > + */ > > > +YamlObject *YamlObject::add(std::string key, std::unique_ptr<YamlObject> child) > > > +{ > > > + if (type_ == Type::Empty) > > > + type_ = Type::Dictionary; > > > + > > > + if (type_ != Type::Dictionary) > > > + return nullptr; > > > + > > > + if (dictionary_.find(key) != dictionary_.end()) > > > + return nullptr; > > > + > > > + Value &elem = list_.emplace_back(std::move(key), std::move(child)); > > > + dictionary_.emplace(elem.key, elem.value.get()); > > > + return elem.value.get(); > > > > Since `key` is already a string: > > > > auto [it, inserted] = dictionary_.try_emplace(std::move(key), child.get()); > > if (!inserted) > > return nullptr; > > > > return list_.emplace_back(it->first, std::move(child)).value.get(); > > I assume you meant that this would replace all code from > dictionary_.find() onwards. It seems fine, I'll do that in v2. In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/x86_64-pc-linux-gnu/bits/c++allocator.h:33, from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/allocator.h:46, from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/string:43, from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/locale_classes.h:40, from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/ios_base.h:41, from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/streambuf:43, from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/streambuf_iterator.h:35, from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/iterator:66, from ../../include/libcamera/internal/yaml_parser.h:10, from ../../src/libcamera/yaml_parser.cpp:8: /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/new_allocator.h: In instantiation of ‘void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = libcamera::YamlObject::Value; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value]’: /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/alloc_traits.h:538:17: required from ‘static void std::allocator_traits<std::allocator<_CharT> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = libcamera::YamlObject::Value; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value; allocator_type = std::allocator<libcamera::YamlObject::Value>]’ /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/vector.tcc:117:30: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value; _Alloc = std::allocator<libcamera::YamlObject::Value>; reference = libcamera::YamlObject::Value&]’ ../../src/libcamera/yaml_parser.cpp:491:27: required from here /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/new_allocator.h:191:11: error: cannot bind rvalue reference of type ‘std::string&&’ {aka ‘std::__cxx11::basic_string<char>&&’} to lvalue of type ‘const std::__cxx11::basic_string<char>’ 191 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../include/libcamera/internal/yaml_parser.h:31:37: note: initializing argument 1 of ‘libcamera::YamlObject::Value::Value(std::string&&, std::unique_ptr<libcamera::YamlObject>&&)’ 31 | Value(std::string &&k, std::unique_ptr<YamlObject> &&v) | ~~~~~~~~~~~~~~^ I'll drop the rvalue reference from the YamlObject::Value constructor. > > > +} > > > + > > > #ifndef __DOXYGEN__ > > > > > > class YamlParserContext
2026. 01. 19. 1:07 keltezéssel, Laurent Pinchart írta: > On Tue, Jan 13, 2026 at 04:03:55PM +0200, Laurent Pinchart wrote: >> On Tue, Jan 13, 2026 at 10:43:18AM +0100, Barnabás Pőcze wrote: >>> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: >>>> Add YamlObject::add() functions to add children to a list or dictionary >>>> object. This will be used by the YamlParserContext to replace direct >>>> access to YamlObject member variables to decouple the two classes. >>>> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>>> --- >>>> include/libcamera/internal/yaml_parser.h | 3 ++ >>>> src/libcamera/yaml_parser.cpp | 56 ++++++++++++++++++++++++ >>>> 2 files changed, 59 insertions(+) >>>> >>>> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h >>>> index c98fe003c877..ede2c3de687c 100644 >>>> --- a/include/libcamera/internal/yaml_parser.h >>>> +++ b/include/libcamera/internal/yaml_parser.h >>>> @@ -196,6 +196,9 @@ public: >>>> bool contains(std::string_view key) const; >>>> const YamlObject &operator[](std::string_view key) const; >>>> >>>> + YamlObject *add(std::unique_ptr<YamlObject> child); >>>> + YamlObject *add(std::string key, std::unique_ptr<YamlObject> child); >>>> + >>>> private: >>>> LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject) >>>> >>>> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp >>>> index 9b61d3e8fc50..7462e2d74179 100644 >>>> --- a/src/libcamera/yaml_parser.cpp >>>> +++ b/src/libcamera/yaml_parser.cpp >>>> @@ -448,6 +448,62 @@ const YamlObject &YamlObject::operator[](std::string_view key) const >>>> return *iter->second; >>>> } >>>> >>>> +/** >>>> + * \brief Add a child object to a list >>>> + * \param[in] child The child object >>>> + * >>>> + * Append the \a child node as the last element of this node's children list. >>>> + * This node must be empty, in which case it is converted to the Type::List >>>> + * type, or be a list. Otherwise, the \a child is discarded and the function >>>> + * returns a nullptr. >>>> + * >>>> + * \return A pointer to the child object if successfully added, nullptr >>>> + * otherwise >>>> + */ >>>> +YamlObject *YamlObject::add(std::unique_ptr<YamlObject> child) >>>> +{ >>>> + if (type_ == Type::Empty) >>>> + type_ = Type::List; >>>> + >>>> + if (type_ != Type::List) >>>> + return nullptr; >>>> + >>>> + Value &elem = list_.emplace_back(std::string{}, std::move(child)); >>>> + return elem.value.get(); >>>> +} >>>> + >>>> +/** >>>> + * \brief Add a child object to a dictionary >>>> + * \param[in] key The dictionary key >>>> + * \param[in] child The child object >>>> + * >>>> + * Add the \a child node with the given \a key to this node's children. This >>>> + * node must be empty, in which case it is converted to the Type::Dictionary >>>> + * type, or be a dictionary. Otherwise, the \a child is discarded and the >>>> + * function returns a nullptr. >>>> + * >>>> + * Keys are unique. If a child with the same \a key already exist, the \a child >>>> + * is discarded and the function returns a nullptr. >>> >>> I'm wondering if other behaviours, such as overwriting the child object make sense. >>> Any thoughts? Or are these very specifically here to work with YamlParserContext? >> >> There should be no duplicate keys in YAML so both behaviours should work >> fine for this use case. I don't know at this point which behaviour would >> be the most sensible for the majority of use cases. >> >>>> + * >>>> + * \return A pointer to the child object if successfully added, nullptr >>>> + * otherwise >>>> + */ >>>> +YamlObject *YamlObject::add(std::string key, std::unique_ptr<YamlObject> child) >>>> +{ >>>> + if (type_ == Type::Empty) >>>> + type_ = Type::Dictionary; >>>> + >>>> + if (type_ != Type::Dictionary) >>>> + return nullptr; >>>> + >>>> + if (dictionary_.find(key) != dictionary_.end()) >>>> + return nullptr; >>>> + >>>> + Value &elem = list_.emplace_back(std::move(key), std::move(child)); >>>> + dictionary_.emplace(elem.key, elem.value.get()); >>>> + return elem.value.get(); >>> >>> Since `key` is already a string: >>> >>> auto [it, inserted] = dictionary_.try_emplace(std::move(key), child.get()); >>> if (!inserted) >>> return nullptr; >>> >>> return list_.emplace_back(it->first, std::move(child)).value.get(); >> >> I assume you meant that this would replace all code from >> dictionary_.find() onwards. It seems fine, I'll do that in v2. > > In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/x86_64-pc-linux-gnu/bits/c++allocator.h:33, > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/allocator.h:46, > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/string:43, > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/locale_classes.h:40, > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/ios_base.h:41, > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/streambuf:43, > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/streambuf_iterator.h:35, > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/iterator:66, > from ../../include/libcamera/internal/yaml_parser.h:10, > from ../../src/libcamera/yaml_parser.cpp:8: > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/new_allocator.h: In instantiation of ‘void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = libcamera::YamlObject::Value; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value]’: > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/alloc_traits.h:538:17: required from ‘static void std::allocator_traits<std::allocator<_CharT> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = libcamera::YamlObject::Value; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value; allocator_type = std::allocator<libcamera::YamlObject::Value>]’ > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/vector.tcc:117:30: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value; _Alloc = std::allocator<libcamera::YamlObject::Value>; reference = libcamera::YamlObject::Value&]’ > ../../src/libcamera/yaml_parser.cpp:491:27: required from here > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/new_allocator.h:191:11: error: cannot bind rvalue reference of type ‘std::string&&’ {aka ‘std::__cxx11::basic_string<char>&&’} to lvalue of type ‘const std::__cxx11::basic_string<char>’ > 191 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../../include/libcamera/internal/yaml_parser.h:31:37: note: initializing argument 1 of ‘libcamera::YamlObject::Value::Value(std::string&&, std::unique_ptr<libcamera::YamlObject>&&)’ > 31 | Value(std::string &&k, std::unique_ptr<YamlObject> &&v) > | ~~~~~~~~~~~~~~^ > > I'll drop the rvalue reference from the YamlObject::Value constructor. Oops, admittedly I have not tested it. But then list_.emplace_back(std::string(it->first), std::move(child)).value.get(); should work if it wants an rvalue reference. > >>>> +} >>>> + >>>> #ifndef __DOXYGEN__ >>>> >>>> class YamlParserContext > > -- > Regards, > > Laurent Pinchart
On Mon, Jan 19, 2026 at 09:15:18AM +0100, Barnabás Pőcze wrote: > 2026. 01. 19. 1:07 keltezéssel, Laurent Pinchart írta: > > On Tue, Jan 13, 2026 at 04:03:55PM +0200, Laurent Pinchart wrote: > >> On Tue, Jan 13, 2026 at 10:43:18AM +0100, Barnabás Pőcze wrote: > >>> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: > >>>> Add YamlObject::add() functions to add children to a list or dictionary > >>>> object. This will be used by the YamlParserContext to replace direct > >>>> access to YamlObject member variables to decouple the two classes. > >>>> > >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>> --- > >>>> include/libcamera/internal/yaml_parser.h | 3 ++ > >>>> src/libcamera/yaml_parser.cpp | 56 ++++++++++++++++++++++++ > >>>> 2 files changed, 59 insertions(+) > >>>> > >>>> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h > >>>> index c98fe003c877..ede2c3de687c 100644 > >>>> --- a/include/libcamera/internal/yaml_parser.h > >>>> +++ b/include/libcamera/internal/yaml_parser.h > >>>> @@ -196,6 +196,9 @@ public: > >>>> bool contains(std::string_view key) const; > >>>> const YamlObject &operator[](std::string_view key) const; > >>>> > >>>> + YamlObject *add(std::unique_ptr<YamlObject> child); > >>>> + YamlObject *add(std::string key, std::unique_ptr<YamlObject> child); > >>>> + > >>>> private: > >>>> LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject) > >>>> > >>>> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp > >>>> index 9b61d3e8fc50..7462e2d74179 100644 > >>>> --- a/src/libcamera/yaml_parser.cpp > >>>> +++ b/src/libcamera/yaml_parser.cpp > >>>> @@ -448,6 +448,62 @@ const YamlObject &YamlObject::operator[](std::string_view key) const > >>>> return *iter->second; > >>>> } > >>>> > >>>> +/** > >>>> + * \brief Add a child object to a list > >>>> + * \param[in] child The child object > >>>> + * > >>>> + * Append the \a child node as the last element of this node's children list. > >>>> + * This node must be empty, in which case it is converted to the Type::List > >>>> + * type, or be a list. Otherwise, the \a child is discarded and the function > >>>> + * returns a nullptr. > >>>> + * > >>>> + * \return A pointer to the child object if successfully added, nullptr > >>>> + * otherwise > >>>> + */ > >>>> +YamlObject *YamlObject::add(std::unique_ptr<YamlObject> child) > >>>> +{ > >>>> + if (type_ == Type::Empty) > >>>> + type_ = Type::List; > >>>> + > >>>> + if (type_ != Type::List) > >>>> + return nullptr; > >>>> + > >>>> + Value &elem = list_.emplace_back(std::string{}, std::move(child)); > >>>> + return elem.value.get(); > >>>> +} > >>>> + > >>>> +/** > >>>> + * \brief Add a child object to a dictionary > >>>> + * \param[in] key The dictionary key > >>>> + * \param[in] child The child object > >>>> + * > >>>> + * Add the \a child node with the given \a key to this node's children. This > >>>> + * node must be empty, in which case it is converted to the Type::Dictionary > >>>> + * type, or be a dictionary. Otherwise, the \a child is discarded and the > >>>> + * function returns a nullptr. > >>>> + * > >>>> + * Keys are unique. If a child with the same \a key already exist, the \a child > >>>> + * is discarded and the function returns a nullptr. > >>> > >>> I'm wondering if other behaviours, such as overwriting the child object make sense. > >>> Any thoughts? Or are these very specifically here to work with YamlParserContext? > >> > >> There should be no duplicate keys in YAML so both behaviours should work > >> fine for this use case. I don't know at this point which behaviour would > >> be the most sensible for the majority of use cases. > >> > >>>> + * > >>>> + * \return A pointer to the child object if successfully added, nullptr > >>>> + * otherwise > >>>> + */ > >>>> +YamlObject *YamlObject::add(std::string key, std::unique_ptr<YamlObject> child) > >>>> +{ > >>>> + if (type_ == Type::Empty) > >>>> + type_ = Type::Dictionary; > >>>> + > >>>> + if (type_ != Type::Dictionary) > >>>> + return nullptr; > >>>> + > >>>> + if (dictionary_.find(key) != dictionary_.end()) > >>>> + return nullptr; > >>>> + > >>>> + Value &elem = list_.emplace_back(std::move(key), std::move(child)); > >>>> + dictionary_.emplace(elem.key, elem.value.get()); > >>>> + return elem.value.get(); > >>> > >>> Since `key` is already a string: > >>> > >>> auto [it, inserted] = dictionary_.try_emplace(std::move(key), child.get()); > >>> if (!inserted) > >>> return nullptr; > >>> > >>> return list_.emplace_back(it->first, std::move(child)).value.get(); > >> > >> I assume you meant that this would replace all code from > >> dictionary_.find() onwards. It seems fine, I'll do that in v2. > > > > In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/x86_64-pc-linux-gnu/bits/c++allocator.h:33, > > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/allocator.h:46, > > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/string:43, > > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/locale_classes.h:40, > > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/ios_base.h:41, > > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/streambuf:43, > > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/streambuf_iterator.h:35, > > from /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/iterator:66, > > from ../../include/libcamera/internal/yaml_parser.h:10, > > from ../../src/libcamera/yaml_parser.cpp:8: > > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/new_allocator.h: In instantiation of ‘void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = libcamera::YamlObject::Value; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value]’: > > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/alloc_traits.h:538:17: required from ‘static void std::allocator_traits<std::allocator<_CharT> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = libcamera::YamlObject::Value; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value; allocator_type = std::allocator<libcamera::YamlObject::Value>]’ > > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/vector.tcc:117:30: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::unique_ptr<libcamera::YamlObject, std::default_delete<libcamera::YamlObject> >}; _Tp = libcamera::YamlObject::Value; _Alloc = std::allocator<libcamera::YamlObject::Value>; reference = libcamera::YamlObject::Value&]’ > > ../../src/libcamera/yaml_parser.cpp:491:27: required from here > > /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/bits/new_allocator.h:191:11: error: cannot bind rvalue reference of type ‘std::string&&’ {aka ‘std::__cxx11::basic_string<char>&&’} to lvalue of type ‘const std::__cxx11::basic_string<char>’ > > 191 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ../../include/libcamera/internal/yaml_parser.h:31:37: note: initializing argument 1 of ‘libcamera::YamlObject::Value::Value(std::string&&, std::unique_ptr<libcamera::YamlObject>&&)’ > > 31 | Value(std::string &&k, std::unique_ptr<YamlObject> &&v) > > | ~~~~~~~~~~~~~~^ > > > > I'll drop the rvalue reference from the YamlObject::Value constructor. > > Oops, admittedly I have not tested it. But then > > list_.emplace_back(std::string(it->first), std::move(child)).value.get(); > > should work if it wants an rvalue reference. That should work too, but dropping the rvalue reference to the constructor works fine too for our use cases. > >>>> +} > >>>> + > >>>> #ifndef __DOXYGEN__ > >>>> > >>>> class YamlParserContext
diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h index c98fe003c877..ede2c3de687c 100644 --- a/include/libcamera/internal/yaml_parser.h +++ b/include/libcamera/internal/yaml_parser.h @@ -196,6 +196,9 @@ public: bool contains(std::string_view key) const; const YamlObject &operator[](std::string_view key) const; + YamlObject *add(std::unique_ptr<YamlObject> child); + YamlObject *add(std::string key, std::unique_ptr<YamlObject> child); + private: LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject) diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index 9b61d3e8fc50..7462e2d74179 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -448,6 +448,62 @@ const YamlObject &YamlObject::operator[](std::string_view key) const return *iter->second; } +/** + * \brief Add a child object to a list + * \param[in] child The child object + * + * Append the \a child node as the last element of this node's children list. + * This node must be empty, in which case it is converted to the Type::List + * type, or be a list. Otherwise, the \a child is discarded and the function + * returns a nullptr. + * + * \return A pointer to the child object if successfully added, nullptr + * otherwise + */ +YamlObject *YamlObject::add(std::unique_ptr<YamlObject> child) +{ + if (type_ == Type::Empty) + type_ = Type::List; + + if (type_ != Type::List) + return nullptr; + + Value &elem = list_.emplace_back(std::string{}, std::move(child)); + return elem.value.get(); +} + +/** + * \brief Add a child object to a dictionary + * \param[in] key The dictionary key + * \param[in] child The child object + * + * Add the \a child node with the given \a key to this node's children. This + * node must be empty, in which case it is converted to the Type::Dictionary + * type, or be a dictionary. Otherwise, the \a child is discarded and the + * function returns a nullptr. + * + * Keys are unique. If a child with the same \a key already exist, the \a child + * is discarded and the function returns a nullptr. + * + * \return A pointer to the child object if successfully added, nullptr + * otherwise + */ +YamlObject *YamlObject::add(std::string key, std::unique_ptr<YamlObject> child) +{ + if (type_ == Type::Empty) + type_ = Type::Dictionary; + + if (type_ != Type::Dictionary) + return nullptr; + + if (dictionary_.find(key) != dictionary_.end()) + return nullptr; + + Value &elem = list_.emplace_back(std::move(key), std::move(child)); + dictionary_.emplace(elem.key, elem.value.get()); + return elem.value.get(); +} + #ifndef __DOXYGEN__ class YamlParserContext
Add YamlObject::add() functions to add children to a list or dictionary object. This will be used by the YamlParserContext to replace direct access to YamlObject member variables to decouple the two classes. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/internal/yaml_parser.h | 3 ++ src/libcamera/yaml_parser.cpp | 56 ++++++++++++++++++++++++ 2 files changed, 59 insertions(+)