[libcamera-devel] [PATCH v3 1/2] libcamera: Introduce YamlParser as a helper to parse yaml files
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 19 17:44:14 CEST 2022
Hi Han-Lin,
Thank you for the patch.
On Mon, Apr 18, 2022 at 08:09:22PM +0800, Han-Lin Chen via libcamera-devel wrote:
> Introduce YamlParser as a helper to convert contents of a yaml file to
> a tree based structure for easier reading, and to avoid writing parser
> with raw yaml tokens. The class is based on libyaml, and only support
> reading but writing a yaml file.
>
> The interface is inspired by Json::Value class from jsoncpp:
> http://jsoncpp.sourceforge.net/class_json_1_1_value.html
>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
> README.rst | 4 +-
> include/libcamera/internal/meson.build | 1 +
> include/libcamera/internal/yaml_parser.h | 86 +++
> src/libcamera/meson.build | 3 +
> src/libcamera/yaml_parser.cpp | 802 +++++++++++++++++++++++
> 5 files changed, 894 insertions(+), 2 deletions(-)
> create mode 100644 include/libcamera/internal/yaml_parser.h
> create mode 100644 src/libcamera/yaml_parser.cpp
>
> diff --git a/README.rst b/README.rst
> index aae6b79f..b5a2d448 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -60,7 +60,7 @@ Meson Build system: [required]
> pip3 install --user --upgrade meson
>
> for the libcamera core: [required]
> - python3-yaml python3-ply python3-jinja2
> + libyaml-dev python3-yaml python3-ply python3-jinja2
>
> for IPA module signing: [required]
> libgnutls28-dev openssl
> @@ -98,7 +98,7 @@ for tracing with lttng: [optional]
> liblttng-ust-dev python3-jinja2 lttng-tools
>
> for android: [optional]
> - libexif-dev libjpeg-dev libyaml-dev
> + libexif-dev libjpeg-dev
>
> for lc-compliance: [optional]
> libevent-dev
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index c9e055d4..7a780d48 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -42,4 +42,5 @@ libcamera_internal_headers = files([
> 'v4l2_pixelformat.h',
> 'v4l2_subdevice.h',
> 'v4l2_videodevice.h',
> + 'yaml_parser.h',
> ])
> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> new file mode 100644
> index 00000000..e00ac25d
> --- /dev/null
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * yaml_parser.h - libcamera YAML parsing helper
> + */
> +
> +#pragma once
> +
> +#include <map>
You need to include stdio.h for FILE, or cstdio and use std::FILE.
> +#include <string>
> +#include <vector>
> +
> +#include <libcamera/base/class.h>
> +
> +#include <libcamera/geometry.h>
> +
> +namespace libcamera {
> +
> +class YamlParserContext;
> +
> +class YamlObject
> +{
> +public:
> + YamlObject();
> + ~YamlObject();
> +
> + bool isValue() const
> + {
> + return type_ == Value;
> + }
> + bool isList() const
> + {
> + return type_ == List;
> + }
> + bool isDictionary() const
> + {
> + return type_ == Dictionary;
> + }
> +
> +#ifndef __DOXYGEN__
> + template<typename T,
> + typename std::enable_if_t<
> + std::is_same<bool, T>::value ||
> + std::is_same<double, T>::value ||
> + std::is_same<int32_t, T>::value ||
> + std::is_same<uint32_t, T>::value ||
> + std::is_same<std::string, T>::value ||
> + std::is_same<Size, T>::value> * = nullptr>
> +#else
> + template<typename T>
> +#endif
> + T get(const T &defaultValue, bool *ok = nullptr) const;
> +
> + std::size_t size() const;
> + const YamlObject &operator[](std::size_t index) const;
> +
> + bool contains(const std::string &key) const;
> + const YamlObject &operator[](const std::string &key) const;
> + std::vector<std::string> getMemberNames() const;
> +
> +private:
> + LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject)
> +
> + friend class YamlParserContext;
> +
> + enum Type {
> + Dictionary,
> + List,
> + Value,
> + };
> +
> + Type type_;
> +
> + std::string value_;
> + std::vector<std::unique_ptr<YamlObject>> list_;
> + std::map<const std::string, std::unique_ptr<YamlObject>> dictionary_;
> +};
> +
> +class YamlParser final
> +{
> +public:
> + static std::unique_ptr<YamlObject> parse(FILE *fh);
> +};
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 26912ca1..f8e18e03 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -46,6 +46,7 @@ libcamera_sources = files([
> 'v4l2_pixelformat.cpp',
> 'v4l2_subdevice.cpp',
> 'v4l2_videodevice.cpp',
> + 'yaml_parser.cpp',
> ])
>
> libcamera_sources += libcamera_public_headers
> @@ -66,6 +67,7 @@ subdir('proxy')
> libdl = cc.find_library('dl')
> libgnutls = cc.find_library('gnutls', required : true)
> libudev = dependency('libudev', required : false)
> +libyaml = dependency('yaml-0.1', required : true)
>
> if libgnutls.found()
> config_h.set('HAVE_GNUTLS', 1)
> @@ -126,6 +128,7 @@ libcamera_deps = [
> libgnutls,
> liblttng,
> libudev,
> + libyaml,
> ]
>
> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> new file mode 100644
> index 00000000..f4072bb0
> --- /dev/null
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -0,0 +1,802 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * yaml_parser.cpp - libcamera YAML parsing helper
> + */
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include <assert.h>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <yaml.h>
> +
> +/**
> + * \file libcamera/internal/yaml_parser.h
> + * \brief A YAML parser helper
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(YamlParser)
> +
> +namespace {
> +
> +void setOk(bool *ok, bool result)
> +{
> + if (ok)
> + *ok = result;
> +}
> +
> +} /* namespace */
> +
> +/**
> + * \class YamlObject
> + * \brief A class representing the tree structure of the YAML content
> + *
> + * The YamlObject class represents the tree structure of a YAML content. A
s/a YAML content/YAML content/
> + * YamlObject can be a dictionary or list of YamlObjects or a value if a tree
> + * leaf.
> + */
> +YamlObject::YamlObject() = default;
> +
> +/**
> + * \class YamlObject
> + * \brief Destructor of YamlObject.
s/.$//
> + */
> +YamlObject::~YamlObject() = default;
> +
> +/**
> + * \fn YamlObject::isValue()
> + * \brief Return whether the YamlObject is a value
> + *
> + * \return True if the YamlObject is a value, false otherwise
> + */
> +
> +/**
> + * \fn YamlObject::isList()
> + * \brief Return whether the YamlObject is a list
> + *
> + * \return True if the YamlObject is a list, false otherwise
> + */
> +
> +/**
> + * \fn YamlObject::isDictionary()
> + * \brief Return whether the YamlObject is a dictionary
> + *
> + * \return True if the YamlObject is a dictionary, false otherwise
> + */
> +
> +/**
> + * \fn template<typename T> YamlObject::get<T>(
> + * const T &defaultValue, bool *ok) const
> + * \brief Parse the YamlObject as a \a T value
> + * \param[in] defaultValue The default value when failing to parse
> + * \param[out] ok The result of whether the parse succeeded
> + *
> + * This function parses the value of the YamlObject as a \a T object, and
> + * returns the value. If parsing fails (usually because the YamlObject doesn't
> + * store a \a T value), the \a defaultValue is returned, and \a ok is set to
> + * false. Otherwise, the YamlObject boolean value is returned, and \a ok is set
s/boolean //
> + * to true.
> + *
> + * The \a ok pointer is optional and can be a nullptr if the caller doesn't
> + * need to know if parsing succeeded.
> + *
> + * \return Value as a bool type
> + */
> +
> +#ifndef __DOXYGEN__
> +
> +template<>
> +bool YamlObject::get(const bool &defaultValue, bool *ok) const
> +{
> + setOk(ok, false);
> +
> + if (type_ != Value)
> + return defaultValue;
> +
> + if (value_ == "true") {
> + setOk(ok, true);
> + return true;
> + } else if (value_ == "false") {
> + setOk(ok, true);
> + return false;
> + }
> +
> + return defaultValue;
> +}
> +
> +template<>
> +int32_t YamlObject::get(const int32_t &defaultValue, bool *ok) const
> +{
> + setOk(ok, false);
> +
> + if (type_ != Value)
> + return defaultValue;
> +
> + if (value_ == "")
> + return defaultValue;
> +
> + char *end;
> +
> + errno = 0;
> + int32_t value = std::strtol(value_.c_str(), &end, 10);
> +
> + if ('\0' != *end || errno == ERANGE)
> + return defaultValue;
> +
> + setOk(ok, true);
> + return value;
> +}
> +
> +template<>
> +uint32_t YamlObject::get(const uint32_t &defaultValue, bool *ok) const
> +{
> + setOk(ok, false);
> +
> + if (type_ != Value)
> + return defaultValue;
> +
> + if (value_ == "")
> + return defaultValue;
> +
> + char *end;
> +
> + errno = 0;
> + uint32_t value = std::strtoul(value_.c_str(), &end, 10);
> +
> + if ('\0' != *end || errno == ERANGE)
> + return defaultValue;
> +
> + setOk(ok, true);
> + return value;
> +}
> +
> +template<>
> +double YamlObject::get(const double &defaultValue, bool *ok) const
> +{
> + setOk(ok, false);
> +
> + if (type_ != Value)
> + return defaultValue;
> +
> + if (value_ == "")
> + return defaultValue;
> +
> + char *end;
> +
> + errno = 0;
> + double value = std::strtod(value_.c_str(), &end);
> +
> + if ('\0' != *end || errno == ERANGE)
> + return defaultValue;
> +
> + setOk(ok, true);
> + return value;
> +}
> +
> +template<>
> +std::string YamlObject::get(const std::string &defaultValue, bool *ok) const
> +{
> + setOk(ok, false);
> +
> + if (type_ != Value)
> + return defaultValue;
> +
> + setOk(ok, true);
> + return value_;
> +}
> +
> +template<>
> +Size YamlObject::get(const Size &defaultValue, bool *ok) const
> +{
> + setOk(ok, false);
> +
> + if (type_ != List)
> + return defaultValue;
> +
> + if (list_.size() != 2)
> + return defaultValue;
> +
> + /*
> + * Add a local variable to validate of each diemension in case
s/diemension/dimension/
> + * that ok == nullptr.
> + */
> + bool valid;
> + uint32_t width = list_[0]->get<uint32_t>(0, &valid);
> + if (!valid)
> + return defaultValue;
> +
> + uint32_t height = list_[1]->get<uint32_t>(0, &valid);
> + if (!valid)
> + return defaultValue;
> +
> + setOk(ok, true);
> + return Size(width, height);
> +}
> +
> +#endif /* __DOXYGEN__ */
> +
> +/**
> + * \fn YamlObject::size()
> + * \brief Retrieve the number of elements in a list YamlObject
> + *
> + * This function retrieves the size of the YamlObject, defined as the number of
> + * child elements it contains. Only YamlObject instances of list type have a
> + * size, calling this function on other types of instances is invalid and
> + * results in undefined behaviour.
> + *
> + * \return The size of the YamlObject
> + */
> +std::size_t YamlObject::size() const
> +{
> + assert(type_ == List);
> +
> + return list_.size();
> +}
> +
> +/**
> + * \fn YamlObject::operator[](std::size_t index) const
> + * \brief Retrieve the element from list YamlObject by index
> + *
> + * This function retrieves an element of the YamlObject. Only YamlObject
> + * instances of list type associates elements with index, calling this function
s/associates/associate/
> + * on other types of instances is invalid and results in undefined behaviour.
> + *
> + * \return The YamlObject as an element of the list
> + */
> +const YamlObject &YamlObject::operator[](std::size_t index) const
> +{
> + assert(type_ == List);
> +
> + return *list_[index];
> +}
> +
> +/**
> + * \fn YamlObject::contains()
> + * \brief Check if an element of a dictionary exists
> + *
> + * This function check if the YamlObject contains an element. Only YamlObject
> + * instances of dictionary type associates elements with names, calling this
s/associates/associate/
> + * function on other types of instances is invalid and results in undefined
> + * behaviour.
> + *
> + * \return True if an element exists, false otherwise
> + */
> +bool YamlObject::contains(const std::string &key) const
> +{
> + assert(type_ == Dictionary);
> +
> + if (dictionary_.find(key) == dictionary_.end())
> + return false;
> +
> + return true;
> +}
> +
> +/**
> + * \fn YamlObject::getMemberNames()
> + * \brief Retrieve all member names of the dictionary
> + *
> + * This function retrieve member names of a YamlObject. Only YamlObject
> + * instances of dictionary type associates elements with names, calling this
s/associates/associate/
> + * function on other types of instances is invalid and results in undefined
> + * behaviour.
I'd add a comment here with
* \todo Replace this function with an iterator-based API
> + *
> + * \return A vector of string as the member names
> + */
> +std::vector<std::string> YamlObject::getMemberNames() const
> +{
> + assert(type_ == Dictionary);
> +
> + std::vector<std::string> memberNames;
> + for (auto &[key, _] : dictionary_)
> + memberNames.push_back(key);
> +
> + return memberNames;
> +}
> +
> +/**
> + * \fn YamlObject::operator[](const std::string &key) const
> + * \brief Retrieve a member by name from the dictionary
> + *
> + * This function retrieve a memberof a YamlObject by name. Only YamlObject
s/memberof/member of/
> + * instances of dictionary type associates elements with names, calling this
s/associates/associate/
> + * function on other types of instances is invalid and results in undefined
> + * behaviour.
> + *
> + * \return A YamlObject as a member
* \return The YamlObject corresponding to the \a key member
> + */
> +const YamlObject &YamlObject::operator[](const std::string &key) const
> +{
> + assert(type_ == Dictionary);
> + assert(contains(key));
> +
> + auto iter = dictionary_.find(key);
> + return *iter->second;
> +}
> +
> +class YamlParserContext
> +{
> +public:
> + YamlParserContext();
> + ~YamlParserContext();
> +
> + using ItemParser = const std::function<int()> &;
> + using RecordParser = const std::function<int(const std::string &)> &;
> +
> + int initParser(FILE *fh);
> + void release();
> +
> + int consumeDocumentStart();
> + int consumeDocumentEnd();
> +
> + int parseString(std::string &value);
> + int parseList(ItemParser parseItem);
> + int parseDictionary(RecordParser parseRecord);
> +
> + int parseNextYamlObject(YamlObject &yamlObject);
> +
> +private:
> + int nextEvent(yaml_event_t &event);
> + int consume(yaml_event_type_t);
> +
> + int parseDictionaryOrList(bool isDictionary,
> + const std::function<int()> &parseItem);
> +
> + bool nextEventLoaded_;
> + yaml_event_t nextEvent_;
> +
> + bool parserValid_;
> + yaml_parser_t parser_;
> +};
> +
> +/**
> + * \class YamlParserContext
As this is an internal class, could you exclude it from the doxygen
generated content ?
> + * \brief Class for YamlParser parsing and context data
> + *
> + * The YamlParserContext class stores the internal yaml_parser_t and provides
> + * helper functions to do event-based parsing for YAML files.
> + */
> +YamlParserContext::YamlParserContext()
> + : nextEventLoaded_(false), parserValid_(false)
> +{
> +}
> +
> +/**
> + * \class YamlParserContext
> + * \brief Destructor of YamlParserContext.
s/.*//
> + */
> +YamlParserContext::~YamlParserContext()
> +{
> + release();
> +}
> +
> +/**
> + * \fn YamlParserContext::initParser()
> + * \brief Initialize a parser with an opened file for parsing
As this function initializes the whole context, I'd call it init(), and
document it as
* \brief Initialize the parser context with an open file
> + * \param[in] fh The YAML file to parse
> + *
> + * Prior to parsing the YAML content, the YamlParser must be initialized with
s/YamlParser/YamlParserContext/
> + * an opened FILE to create an internal parser. The FILE need to stay valid
s/need/needs/
> + * during the process. The release() must be called after use.
s/release()/release() function/
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to initialize
s/is failed/has failed/
> + */
> +int YamlParserContext::initParser(FILE *fh)
> +{
> + /* yaml_parser_initialize returns 1 when succeeded */
s/when succeeded/on success/ (or "when it succeeds")
> + if (!yaml_parser_initialize(&parser_)) {
> + LOG(YamlParser, Error) << "Failed to initialize YAML parser";
> + return -EINVAL;
> + }
> + parserValid_ = true;
> + yaml_parser_set_input_file(&parser_, fh);
> +
> + return 0;
> +}
> +
> +/**
> + * \fn YamlParserContext::release()
> + * \brief Release the internal parser
> + *
> + * After parsing the YAML content, The YamlParser::release() should be called
> + * to release the internal parser.
> + */
> +void YamlParserContext::release()
As the parser context isn't reused, I think you can move the contents of
this function to the destructor. The comment above that references the
release() function can be dropped, and that will simplify
YamlParser::parse().
> +{
> + if (nextEventLoaded_) {
> + yaml_event_delete(&nextEvent_);
> + nextEventLoaded_ = false;
> + }
> +
> + if (parserValid_) {
> + parserValid_ = false;
> + yaml_parser_delete(&parser_);
Swap the two lines to match nextEventLoaded_ ?
> + }
> +}
> +
> +/**
> + * \fn YamlParserContext::consume()
> + * \brief Consume the next event with an expected event type
> + *
No need for a blank line here.
> + * \param[in] type The expected event type to consume
> + *
> + * Consume next event and check whether next event has an expected type.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser failed to initialize
I think this should be
* \retval -EINVAL The \a type doesn't match the next event
> + */
> +int YamlParserContext::consume(yaml_event_type_t type)
> +{
> + yaml_event_t event;
> + int ret = nextEvent(event);
> + if (ret)
> + return ret;
> +
> + if (event.type != type) {
> + LOG(YamlParser, Error)
> + << "Expect event: " << type
> + << " but get " << event.type
> + << " at line: " << event.start_mark.line
> + << " column: " << event.start_mark.column;
> + return -EINVAL;
> + }
> +
> + yaml_event_delete(&event);
> + nextEventLoaded_ = false;
> +
> + return 0;
> +}
> +
> +/**
> + * \typedef YamlParserContext::ItemParser
> + * \brief The functor to handle an item in a YAML list
> + */
> +
> +/**
> + * \typedef YamlParserContext::RecordParser
> + * \brief The functor to handle an item in a YAML dictionary
> + */
> +
> +/**
> + * \fn YamlParserContext::consumeDocumentStart()
> + * \brief Consume start of a YAML document
> + *
> + * Check YAML start of a YAML document. The function should be called and
> + * checked before parsing any content of the YAML file.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to validate start of a YAML file
> + */
> +int YamlParserContext::consumeDocumentStart()
> +{
> + if (consume(YAML_STREAM_START_EVENT))
> + return -EINVAL;
> +
> + if (consume(YAML_DOCUMENT_START_EVENT))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> + * \fn YamlParserContext::consumeDocumentEnd()
> + * \brief Consume end of a YAML document
> + *
> + * Check YAML end of a YAML document. The function should be called and
> + * checked after parsing any content of the YAML file.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to validate end of a YAML file
> + */
> +int YamlParserContext::consumeDocumentEnd()
> +{
> + if (consume(YAML_DOCUMENT_END_EVENT))
> + return -EINVAL;
> +
> + if (consume(YAML_STREAM_END_EVENT))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> + * \fn YamlParserContext::parseString()
> + * \brief Parse scalar and read its content as a string
> + * \param[in] value The string reference to fill value
> + *
> + * A helper function to peek the next event, check whether it's scalar type
> + * and read its content as a string.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to initialize
> + */
> +int YamlParserContext::parseString(std::string &value)
> +{
> + yaml_event_t event;
> + int ret = nextEvent(event);
> + if (ret)
> + return ret;
> +
> + if (event.type != YAML_SCALAR_EVENT) {
> + LOG(YamlParser, Error) << "Expect scalar at line: "
> + << event.start_mark.line
> + << " column: "
> + << event.start_mark.column;
> + return -EINVAL;
> + }
> +
> + value.assign(reinterpret_cast<char *>(event.data.scalar.value),
> + event.data.scalar.length);
> +
> + consume(YAML_SCALAR_EVENT);
> +
> + return 0;
> +}
> +
> +/**
> + * \fn YamlParserContext::parseList()
> + * \brief Parse a list with an callback function to parse each item in the list
> + * \param[in] parseItem The functor to parse a single item in the list
> + *
> + * Start to parse a list from the current position and call back to parseItem
> + * for parsing each item in the list. The parseItem should return 0 on success
> + * or a negative error code otherwise.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to parse the list
> + */
> +int YamlParserContext::parseList(ItemParser parseItem)
> +{
> + return parseDictionaryOrList(false, parseItem);
> +}
> +
> +/**
> + * \fn YamlParserContext::parseDictionary()
> + * \brief Parse a dictionary with an callback function to parse each item
> + * \param[in] parseRecord The functor to parse a single item in the dictionary
> + *
> + * Start to parse a dictionary from the current position and call back to
> + * parseRecord to parse value by a string argument as the key to the value.
> + * The parseRecord should return 0 on success or a negative error code
> + * otherwise.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to parse the dictionary
> + */
> +int YamlParserContext::parseDictionary(RecordParser parseRecord)
> +{
> + auto parseItem = [this, &parseRecord]() {
> + std::string key;
> + int ret = parseString(key);
> + if (ret)
> + return -EINVAL;
> +
> + return parseRecord(key);
> + };
> +
> + return parseDictionaryOrList(true, parseItem);
> +}
> +
> +/**
> + * \fn YamlParserContext::parseDictionaryOrList()
> + * \brief A helper function to abstract common part of parsing dictionary or list
> + *
> + * \param[in] isDictionary True for parsing a dictionary, and false for a list
> + * \param[in] parseItem The callback to handle an item
> + *
> + * A helper function to abstract parsing a item from a dictionary or a list.
> + * The differences of them in a YAML event stream are:
> + *
> + * 1. The start and end event type are different
> + * 2. There is a leading scalar string as key in the items of a dictionary
> + *
> + * The caller should handle the leading key string in its callback parseItem
> + * when it's a dictionary.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to initialize
> + */
> +int YamlParserContext::parseDictionaryOrList(bool isDictionary,
> + const std::function<int()> &parseItem)
> +{
> + yaml_event_type_t startEventType = YAML_SEQUENCE_START_EVENT;
> + yaml_event_type_t endEventType = YAML_SEQUENCE_END_EVENT;
> +
> + if (isDictionary) {
> + startEventType = YAML_MAPPING_START_EVENT;
> + endEventType = YAML_MAPPING_END_EVENT;
> + }
> +
> + if (consume(startEventType))
> + return -EINVAL;
> +
> + /*
> + * Add a safety counter to make sure we don't loop indefinitely in case
> + * the configuration file is malformed.
> + */
> + unsigned int sentinel = 1000;
> + yaml_event_t event;
> + do {
> + int ret = nextEvent(event);
> + if (ret)
> + return ret;
> +
> + if (event.type == endEventType)
> + return consume(endEventType);
> +
> + ret = parseItem();
> + if (ret)
> + return ret;
> +
> + --sentinel;
> + } while (sentinel);
> +
> + if (!sentinel)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> + * \fn YamlParserContext::nextEvent()
> + * \brief Peek the next event
> + *
> + * \param[in] event The event reference to fill information
> + *
> + * Peek the next event in the current YAML event stream, and return -EINVAL when
> + * there is no more event.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The parser is failed to initialize
> + */
> +int YamlParserContext::nextEvent(yaml_event_t &event)
> +{
> + if (nextEventLoaded_) {
> + event = nextEvent_;
> + return 0;
> + }
I'm not a big fan of this mechanism. Wouldn't it be possible to always
consume the next event, possibly returning it from this function as a
unique_ptr, and passing it down the call stack where needed ? For
instance the event returned by nextEvent() in
YamlParserContext::parseNextYamlObject() would be passed to
parseString(). I think it would simplify the code, as some checks could
also be removed (for instance the event type check in parseString(), as
the type is checked in parseNextYamlObject() already).
> +
> + /* yaml_parser_parse returns when it succeeds */
> + if (1 != yaml_parser_parse(&parser_, &nextEvent_)) {
> + return -EINVAL;
> + }
No need for curly braces.
> +
> + nextEventLoaded_ = true;
> + event = nextEvent_;
> +
> + return 0;
> +}
> +
> +/**
> + * \fn YamlParserContext::parseNextYamlObject()
> + * \brief Parse next YAML event and read it as a YamlObject
> + * \param[in] yamlObject The result of YamlObject
> + *
> + * Parse next YAML event by peeking next event and parse
> + * them separately as a value, list or dictionary.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL Fail to parse the YAML file.
> + */
> +int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject)
> +{
> + yaml_event_t event;
> +
> + if (nextEvent(event))
> + return -EINVAL;
> +
> + if (event.type == YAML_SCALAR_EVENT) {
It looks like a switch/case would be a good replacement for the if's.
> + yamlObject.type_ = YamlObject::Value;
> + parseString(yamlObject.value_);
> + return 0;
> + }
> +
> + if (event.type == YAML_SEQUENCE_START_EVENT) {
> + yamlObject.type_ = YamlObject::List;
> + auto &list = yamlObject.list_;
> + auto handler = [this, &list]() {
> + list.emplace_back(new YamlObject());
> + return parseNextYamlObject(*list.back());
> + };
> + return parseList(handler);
> + }
> +
> + if (event.type == YAML_MAPPING_START_EVENT) {
> + yamlObject.type_ = YamlObject::Dictionary;
> + auto &dictionary = yamlObject.dictionary_;
> + auto handler = [this, &dictionary](const std::string &key) {
> + dictionary[key].reset(new YamlObject());
> + return parseNextYamlObject(*dictionary[key]);
> + };
> + return parseDictionary(handler);
> + }
> +
> + LOG(YamlParser, Error) << "Invalid YAML file";
> + return -EINVAL;
> +}
> +
> +/**
> + * \class YamlParser
> + * \brief A helper class for parsing YAML files.
s/files.$/files/
> + *
> + * The YamlParser class provides an easy interface to parse the contents of a
> + * YAML file int a tree fo YamlObject instances.
s/tree fo/tree of/
> + *
> + * Example usage 1:
> + * The following code illustrates how to parse the following YAML file:
> + *
> + * name:
> + * "John"
> + * numbers:
> + * - 1
> + * - 2
You need to put this in a quote block, or doxygen will likely misrender
it.
> + *
> + * \code
> + *
> + * YamlObject root;
> + * std::unique_ptr<YamlObject> root = YamlParser::parse(fh);
> + * if (!root)
> + * return;
> + *
> + * if (!root->isDictionary())
> + * return;
> + *
> + * std::string name = (*root)["name"];
std::string isn't the right return type.
> + * std::cout << name.get<std::string>("");
> + *
> + * const YamlObject &numbers = (*root)["numbers"];
> + * if (!numbers.isList())
> + * return;
> + *
> + * for (int i = 0; i < numbers.size; i++)
> + * std::cout << numbers[i].get<int32_t>(0);
for (std::size_t i = 0; i < numbers.size(); i++)
std::cout << numbers[i].get<int32_t>(0) << std::endl;
It would be nice to try and compile that code snippet.
> + *
> + * \endcode
> + *
> + * The YamlParser::parse() function accepts an open FILE and initializes an
> + * internal parse.
Let's not document the internal implementation details in the public
documentation:
* The YamlParser::parse() function takes open FILE, parses its contents, and
* returns a pointer to a YamlObject corresponding to the root node of the YAML
* document.
> + */
> +
> +/**
> + * \fn YamlParser::parse()
> + * \brief Parse a YAML file as a YamlObject
> + * \param[in] fh The YAML file to parse
> + *
> + * Prior to parsing the YAML content, the function accepts an opened FILE to
> + * create an internal parser.
Same here.
* This function takes open FILE, parses its contents, and returns a pointer to
* a YamlObject corresponding to the root node of the YAML document. The caller
* is responsible for closing the file after parsing completes.
> + *
> + * \return Pointer to result YamlObject on success or nullptr otherwise
> + */
> +std::unique_ptr<YamlObject> YamlParser::parse(FILE *fh)
> +{
> + YamlParserContext context;
> +
> + if (context.initParser(fh))
> + return nullptr;
> +
> + std::unique_ptr<YamlObject> root(new YamlObject());
> +
> + if (context.consumeDocumentStart())
> + goto error;
> +
> + if (context.parseNextYamlObject(*root))
> + goto error;
> +
> + if (context.consumeDocumentEnd())
> + goto error;
> +
> + context.release();
> + return root;
> +
> +error:
> + root.release();
No need for this, the unique_ptr will be destroyed when this function
returns.
> + context.release();
> + return nullptr;
> +}
> +
> +} /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list