[libcamera-devel] [PATCH v3 1/2] libcamera: Introduce YamlParser as a helper to parse yaml files
Hanlin Chen
hanlinchen at chromium.org
Mon Apr 25 16:51:29 CEST 2022
Hi Laurent,
On Mon, Apr 25, 2022 at 7:42 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Han-Lin,
>
> On Wed, Apr 20, 2022 at 09:37:59PM +0800, Hanlin Chen wrote:
> > On Tue, Apr 19, 2022 at 11:44 PM Laurent Pinchart wrote:
> > > 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).
> >
> > Yes, I was struggled with this...
> > If it always consumes the next event, the first event has to pass to
> > parseItem of parseDictionaryOrList(), and thus propagate to all
> > intermediate handlers and to (by recursion) parseNextYamlObject().
>
> I'm not sure to see the problem. Could you give me an example ?
I changed it eventually. Please see the v5 version.
>
> > There was an intermediate version which is the case, but I abandoned
> > it because it seems unclean to me.
> > nextEvent() is added after that to peek the next event for conditional
> > check, and let each item handler consume its start and end token
> > symmetrically.
> >
> > > > +
> > > > + /* 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