[14/36] libcamera: yaml_parser: Un-friend YamlParserContext from YamlObject
diff mbox series

Message ID 20260113000808.15395-15-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • libcamera: Global configuration file improvements
Related show

Commit Message

Laurent Pinchart Jan. 13, 2026, 12:07 a.m. UTC 
YamlParserContext is a friend of the YamlObject class to access private
member variables. Now that YamlObject exposes functions to set a value
and add children, this is not needed anymore. Decouple the two classes.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/yaml_parser.h |  1 -
 src/libcamera/yaml_parser.cpp            | 56 +++++++++---------------
 2 files changed, 21 insertions(+), 36 deletions(-)

Comments

Barnabás Pőcze Jan. 13, 2026, 9:43 a.m. UTC | #1 
2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:
> YamlParserContext is a friend of the YamlObject class to access private
> member variables. Now that YamlObject exposes functions to set a value
> and add children, this is not needed anymore. Decouple the two classes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   include/libcamera/internal/yaml_parser.h |  1 -
>   src/libcamera/yaml_parser.cpp            | 56 +++++++++---------------
>   2 files changed, 21 insertions(+), 36 deletions(-)
> 
> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> index ede2c3de687c..02e39e4cd7a1 100644
> --- a/include/libcamera/internal/yaml_parser.h
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -204,7 +204,6 @@ private:
> 
>   	template<typename T>
>   	friend struct Accessor;
> -	friend class YamlParserContext;
> 
>   	enum class Type {
>   		Dictionary,
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index 7462e2d74179..fa1a487a13dc 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -530,8 +530,8 @@ private:
> 
>   	EventPtr nextEvent();
> 
> -	void readValue(std::string &value, EventPtr event);
> -	int parseDictionaryOrList(YamlObject::Type type,
> +	std::string readValue(EventPtr event);
> +	int parseDictionaryOrList(yaml_event_type_t endEventType,
>   				  const std::function<int(EventPtr event)> &parseItem);
>   	int parseNextYamlObject(YamlObject &yamlObject, EventPtr event);
> 
> @@ -673,22 +673,22 @@ int YamlParserContext::parseContent(YamlObject &yamlObject)
>   /**
>    * \fn YamlParserContext::readValue()
>    * \brief Parse event scalar and fill its content into a string
> - * \param[in] value The string reference to fill value
>    *
>    * A helper function to parse a scalar event as string. The caller needs to
> - * guarantee the event is of scaler type.
> + * guarantee the event is of scalar type.
> + *
> + * \return The scalar event value as a string
>    */
> -void YamlParserContext::readValue(std::string &value, EventPtr event)
> +std::string YamlParserContext::readValue(EventPtr event)
>   {
> -	value.assign(reinterpret_cast<char *>(event->data.scalar.value),
> -		     event->data.scalar.length);
> +	return std::string{ reinterpret_cast<char *>(event->data.scalar.value),
> +			    event->data.scalar.length };
>   }
> 
>   /**
>    * \fn YamlParserContext::parseDictionaryOrList()
>    * \brief A helper function to abstract the common part of parsing dictionary or list
> - *
> - * \param[in] isDictionary True for parsing a dictionary, and false for a list
> + * \param[in] endEventType The YAML end event type (sequence or mapping)
>    * \param[in] parseItem The callback to handle an item
>    *
>    * A helper function to abstract parsing an item from a dictionary or a list.
> @@ -703,13 +703,9 @@ void YamlParserContext::readValue(std::string &value, EventPtr event)
>    * \return 0 on success or a negative error code otherwise
>    * \retval -EINVAL The parser is failed to initialize
>    */
> -int YamlParserContext::parseDictionaryOrList(YamlObject::Type type,
> +int YamlParserContext::parseDictionaryOrList(yaml_event_type_t endEventType,
>   					     const std::function<int(EventPtr event)> &parseItem)
>   {
> -	yaml_event_type_t endEventType = YAML_SEQUENCE_END_EVENT;
> -	if (type == YamlObject::Type::Dictionary)
> -		endEventType = YAML_MAPPING_END_EVENT;
> -
>   	/*
>   	 * Add a safety counter to make sure we don't loop indefinitely in case
>   	 * the YAML file is malformed.
> @@ -751,24 +747,19 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
> 
>   	switch (event->type) {
>   	case YAML_SCALAR_EVENT:
> -		yamlObject.type_ = YamlObject::Type::Value;
> -		readValue(yamlObject.value_, std::move(event));
> +		yamlObject.set(readValue(std::move(event)));
>   		return 0;
> 
>   	case YAML_SEQUENCE_START_EVENT: {
> -		yamlObject.type_ = YamlObject::Type::List;
> -		auto &list = yamlObject.list_;
> -		auto handler = [this, &list](EventPtr evt) {
> -			list.emplace_back(std::string{}, std::make_unique<YamlObject>());
> -			return parseNextYamlObject(*list.back().value, std::move(evt));
> +		auto handler = [this, &yamlObject](EventPtr evt) {
> +			YamlObject *child = yamlObject.add(std::make_unique<YamlObject>());
> +			return parseNextYamlObject(*child, std::move(evt));
>   		};
> -		return parseDictionaryOrList(YamlObject::Type::List, handler);
> +		return parseDictionaryOrList(YAML_SEQUENCE_END_EVENT, handler);
>   	}
> 
>   	case YAML_MAPPING_START_EVENT: {
> -		yamlObject.type_ = YamlObject::Type::Dictionary;
> -		auto &list = yamlObject.list_;
> -		auto handler = [this, &list](EventPtr evtKey) {
> +		auto handler = [this, &yamlObject](EventPtr evtKey) {
>   			/* Parse key */
>   			if (evtKey->type != YAML_SCALAR_EVENT) {
>   				LOG(YamlParser, Error) << "Expect key at line: "
> @@ -778,26 +769,21 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
>   				return -EINVAL;
>   			}
> 
> -			std::string key;
> -			readValue(key, std::move(evtKey));
> +			std::string key = readValue(std::move(evtKey));
> 
>   			/* Parse value */
>   			EventPtr evtValue = nextEvent();
>   			if (!evtValue)
>   				return -EINVAL;
> 
> -			auto &elem = list.emplace_back(std::move(key),
> -						       std::make_unique<YamlObject>());
> -			return parseNextYamlObject(*elem.value, std::move(evtValue));
> +			YamlObject *child = yamlObject.add(std::move(key),
> +							   std::make_unique<YamlObject>());

I think the return value should be checked here and in the list case. Even if only with ASSERT().
A simple duplicate key will cause an issue here.


> +			return parseNextYamlObject(*child, std::move(evtValue));
>   		};
> -		int ret = parseDictionaryOrList(YamlObject::Type::Dictionary, handler);
> +		int ret = parseDictionaryOrList(YAML_MAPPING_END_EVENT, handler);
>   		if (ret)
>   			return ret;
> 
> -		auto &dictionary = yamlObject.dictionary_;
> -		for (const auto &elem : list)
> -			dictionary.emplace(elem.key, elem.value.get());
> -
>   		return 0;
>   	}
> 
> --
> Regards,
> 
> Laurent Pinchart
>
Laurent Pinchart Jan. 13, 2026, 2:05 p.m. UTC | #2 
On Tue, Jan 13, 2026 at 10:43:29AM +0100, Barnabás Pőcze wrote:
> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:
> > YamlParserContext is a friend of the YamlObject class to access private
> > member variables. Now that YamlObject exposes functions to set a value
> > and add children, this is not needed anymore. Decouple the two classes.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >   include/libcamera/internal/yaml_parser.h |  1 -
> >   src/libcamera/yaml_parser.cpp            | 56 +++++++++---------------
> >   2 files changed, 21 insertions(+), 36 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> > index ede2c3de687c..02e39e4cd7a1 100644
> > --- a/include/libcamera/internal/yaml_parser.h
> > +++ b/include/libcamera/internal/yaml_parser.h
> > @@ -204,7 +204,6 @@ private:
> > 
> >   	template<typename T>
> >   	friend struct Accessor;
> > -	friend class YamlParserContext;
> > 
> >   	enum class Type {
> >   		Dictionary,
> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> > index 7462e2d74179..fa1a487a13dc 100644
> > --- a/src/libcamera/yaml_parser.cpp
> > +++ b/src/libcamera/yaml_parser.cpp
> > @@ -530,8 +530,8 @@ private:
> > 
> >   	EventPtr nextEvent();
> > 
> > -	void readValue(std::string &value, EventPtr event);
> > -	int parseDictionaryOrList(YamlObject::Type type,
> > +	std::string readValue(EventPtr event);
> > +	int parseDictionaryOrList(yaml_event_type_t endEventType,
> >   				  const std::function<int(EventPtr event)> &parseItem);
> >   	int parseNextYamlObject(YamlObject &yamlObject, EventPtr event);
> > 
> > @@ -673,22 +673,22 @@ int YamlParserContext::parseContent(YamlObject &yamlObject)
> >   /**
> >    * \fn YamlParserContext::readValue()
> >    * \brief Parse event scalar and fill its content into a string
> > - * \param[in] value The string reference to fill value
> >    *
> >    * A helper function to parse a scalar event as string. The caller needs to
> > - * guarantee the event is of scaler type.
> > + * guarantee the event is of scalar type.
> > + *
> > + * \return The scalar event value as a string
> >    */
> > -void YamlParserContext::readValue(std::string &value, EventPtr event)
> > +std::string YamlParserContext::readValue(EventPtr event)
> >   {
> > -	value.assign(reinterpret_cast<char *>(event->data.scalar.value),
> > -		     event->data.scalar.length);
> > +	return std::string{ reinterpret_cast<char *>(event->data.scalar.value),
> > +			    event->data.scalar.length };
> >   }
> > 
> >   /**
> >    * \fn YamlParserContext::parseDictionaryOrList()
> >    * \brief A helper function to abstract the common part of parsing dictionary or list
> > - *
> > - * \param[in] isDictionary True for parsing a dictionary, and false for a list
> > + * \param[in] endEventType The YAML end event type (sequence or mapping)
> >    * \param[in] parseItem The callback to handle an item
> >    *
> >    * A helper function to abstract parsing an item from a dictionary or a list.
> > @@ -703,13 +703,9 @@ void YamlParserContext::readValue(std::string &value, EventPtr event)
> >    * \return 0 on success or a negative error code otherwise
> >    * \retval -EINVAL The parser is failed to initialize
> >    */
> > -int YamlParserContext::parseDictionaryOrList(YamlObject::Type type,
> > +int YamlParserContext::parseDictionaryOrList(yaml_event_type_t endEventType,
> >   					     const std::function<int(EventPtr event)> &parseItem)
> >   {
> > -	yaml_event_type_t endEventType = YAML_SEQUENCE_END_EVENT;
> > -	if (type == YamlObject::Type::Dictionary)
> > -		endEventType = YAML_MAPPING_END_EVENT;
> > -
> >   	/*
> >   	 * Add a safety counter to make sure we don't loop indefinitely in case
> >   	 * the YAML file is malformed.
> > @@ -751,24 +747,19 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
> > 
> >   	switch (event->type) {
> >   	case YAML_SCALAR_EVENT:
> > -		yamlObject.type_ = YamlObject::Type::Value;
> > -		readValue(yamlObject.value_, std::move(event));
> > +		yamlObject.set(readValue(std::move(event)));
> >   		return 0;
> > 
> >   	case YAML_SEQUENCE_START_EVENT: {
> > -		yamlObject.type_ = YamlObject::Type::List;
> > -		auto &list = yamlObject.list_;
> > -		auto handler = [this, &list](EventPtr evt) {
> > -			list.emplace_back(std::string{}, std::make_unique<YamlObject>());
> > -			return parseNextYamlObject(*list.back().value, std::move(evt));
> > +		auto handler = [this, &yamlObject](EventPtr evt) {
> > +			YamlObject *child = yamlObject.add(std::make_unique<YamlObject>());
> > +			return parseNextYamlObject(*child, std::move(evt));
> >   		};
> > -		return parseDictionaryOrList(YamlObject::Type::List, handler);
> > +		return parseDictionaryOrList(YAML_SEQUENCE_END_EVENT, handler);
> >   	}
> > 
> >   	case YAML_MAPPING_START_EVENT: {
> > -		yamlObject.type_ = YamlObject::Type::Dictionary;
> > -		auto &list = yamlObject.list_;
> > -		auto handler = [this, &list](EventPtr evtKey) {
> > +		auto handler = [this, &yamlObject](EventPtr evtKey) {
> >   			/* Parse key */
> >   			if (evtKey->type != YAML_SCALAR_EVENT) {
> >   				LOG(YamlParser, Error) << "Expect key at line: "
> > @@ -778,26 +769,21 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
> >   				return -EINVAL;
> >   			}
> > 
> > -			std::string key;
> > -			readValue(key, std::move(evtKey));
> > +			std::string key = readValue(std::move(evtKey));
> > 
> >   			/* Parse value */
> >   			EventPtr evtValue = nextEvent();
> >   			if (!evtValue)
> >   				return -EINVAL;
> > 
> > -			auto &elem = list.emplace_back(std::move(key),
> > -						       std::make_unique<YamlObject>());
> > -			return parseNextYamlObject(*elem.value, std::move(evtValue));
> > +			YamlObject *child = yamlObject.add(std::move(key),
> > +							   std::make_unique<YamlObject>());
> 
> I think the return value should be checked here and in the list case. Even if only with ASSERT().
> A simple duplicate key will cause an issue here.

I'll test that and add error checking.

> > +			return parseNextYamlObject(*child, std::move(evtValue));
> >   		};
> > -		int ret = parseDictionaryOrList(YamlObject::Type::Dictionary, handler);
> > +		int ret = parseDictionaryOrList(YAML_MAPPING_END_EVENT, handler);
> >   		if (ret)
> >   			return ret;
> > 
> > -		auto &dictionary = yamlObject.dictionary_;
> > -		for (const auto &elem : list)
> > -			dictionary.emplace(elem.key, elem.value.get());
> > -
> >   		return 0;
> >   	}
> >

Patch
diff mbox series 

diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
index ede2c3de687c..02e39e4cd7a1 100644
--- a/include/libcamera/internal/yaml_parser.h
+++ b/include/libcamera/internal/yaml_parser.h
@@ -204,7 +204,6 @@  private:
 
 	template<typename T>
 	friend struct Accessor;
-	friend class YamlParserContext;
 
 	enum class Type {
 		Dictionary,
diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
index 7462e2d74179..fa1a487a13dc 100644
--- a/src/libcamera/yaml_parser.cpp
+++ b/src/libcamera/yaml_parser.cpp
@@ -530,8 +530,8 @@  private:
 
 	EventPtr nextEvent();
 
-	void readValue(std::string &value, EventPtr event);
-	int parseDictionaryOrList(YamlObject::Type type,
+	std::string readValue(EventPtr event);
+	int parseDictionaryOrList(yaml_event_type_t endEventType,
 				  const std::function<int(EventPtr event)> &parseItem);
 	int parseNextYamlObject(YamlObject &yamlObject, EventPtr event);
 
@@ -673,22 +673,22 @@  int YamlParserContext::parseContent(YamlObject &yamlObject)
 /**
  * \fn YamlParserContext::readValue()
  * \brief Parse event scalar and fill its content into a string
- * \param[in] value The string reference to fill value
  *
  * A helper function to parse a scalar event as string. The caller needs to
- * guarantee the event is of scaler type.
+ * guarantee the event is of scalar type.
+ *
+ * \return The scalar event value as a string
  */
-void YamlParserContext::readValue(std::string &value, EventPtr event)
+std::string YamlParserContext::readValue(EventPtr event)
 {
-	value.assign(reinterpret_cast<char *>(event->data.scalar.value),
-		     event->data.scalar.length);
+	return std::string{ reinterpret_cast<char *>(event->data.scalar.value),
+			    event->data.scalar.length };
 }
 
 /**
  * \fn YamlParserContext::parseDictionaryOrList()
  * \brief A helper function to abstract the common part of parsing dictionary or list
- *
- * \param[in] isDictionary True for parsing a dictionary, and false for a list
+ * \param[in] endEventType The YAML end event type (sequence or mapping)
  * \param[in] parseItem The callback to handle an item
  *
  * A helper function to abstract parsing an item from a dictionary or a list.
@@ -703,13 +703,9 @@  void YamlParserContext::readValue(std::string &value, EventPtr event)
  * \return 0 on success or a negative error code otherwise
  * \retval -EINVAL The parser is failed to initialize
  */
-int YamlParserContext::parseDictionaryOrList(YamlObject::Type type,
+int YamlParserContext::parseDictionaryOrList(yaml_event_type_t endEventType,
 					     const std::function<int(EventPtr event)> &parseItem)
 {
-	yaml_event_type_t endEventType = YAML_SEQUENCE_END_EVENT;
-	if (type == YamlObject::Type::Dictionary)
-		endEventType = YAML_MAPPING_END_EVENT;
-
 	/*
 	 * Add a safety counter to make sure we don't loop indefinitely in case
 	 * the YAML file is malformed.
@@ -751,24 +747,19 @@  int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
 
 	switch (event->type) {
 	case YAML_SCALAR_EVENT:
-		yamlObject.type_ = YamlObject::Type::Value;
-		readValue(yamlObject.value_, std::move(event));
+		yamlObject.set(readValue(std::move(event)));
 		return 0;
 
 	case YAML_SEQUENCE_START_EVENT: {
-		yamlObject.type_ = YamlObject::Type::List;
-		auto &list = yamlObject.list_;
-		auto handler = [this, &list](EventPtr evt) {
-			list.emplace_back(std::string{}, std::make_unique<YamlObject>());
-			return parseNextYamlObject(*list.back().value, std::move(evt));
+		auto handler = [this, &yamlObject](EventPtr evt) {
+			YamlObject *child = yamlObject.add(std::make_unique<YamlObject>());
+			return parseNextYamlObject(*child, std::move(evt));
 		};
-		return parseDictionaryOrList(YamlObject::Type::List, handler);
+		return parseDictionaryOrList(YAML_SEQUENCE_END_EVENT, handler);
 	}
 
 	case YAML_MAPPING_START_EVENT: {
-		yamlObject.type_ = YamlObject::Type::Dictionary;
-		auto &list = yamlObject.list_;
-		auto handler = [this, &list](EventPtr evtKey) {
+		auto handler = [this, &yamlObject](EventPtr evtKey) {
 			/* Parse key */
 			if (evtKey->type != YAML_SCALAR_EVENT) {
 				LOG(YamlParser, Error) << "Expect key at line: "
@@ -778,26 +769,21 @@  int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
 				return -EINVAL;
 			}
 
-			std::string key;
-			readValue(key, std::move(evtKey));
+			std::string key = readValue(std::move(evtKey));
 
 			/* Parse value */
 			EventPtr evtValue = nextEvent();
 			if (!evtValue)
 				return -EINVAL;
 
-			auto &elem = list.emplace_back(std::move(key),
-						       std::make_unique<YamlObject>());
-			return parseNextYamlObject(*elem.value, std::move(evtValue));
+			YamlObject *child = yamlObject.add(std::move(key),
+							   std::make_unique<YamlObject>());
+			return parseNextYamlObject(*child, std::move(evtValue));
 		};
-		int ret = parseDictionaryOrList(YamlObject::Type::Dictionary, handler);
+		int ret = parseDictionaryOrList(YAML_MAPPING_END_EVENT, handler);
 		if (ret)
 			return ret;
 
-		auto &dictionary = yamlObject.dictionary_;
-		for (const auto &elem : list)
-			dictionary.emplace(elem.key, elem.value.get());
-
 		return 0;
 	}