[libcamera-devel] [RFC PATCH v2 03/14] libcamera: yaml_parser: Switch from FILE to File
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Mon Jun 13 06:12:08 CEST 2022
Hi Laurent,
On Sat, Jun 04, 2022 at 09:59:28PM +0300, Laurent Pinchart via libcamera-devel wrote:
> THe FILE object isn't very user-friendly as it requires manual close.
> Replace it with File to provide RAII-style resource management in the
> YamlParser API.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> include/libcamera/internal/yaml_parser.h | 4 +--
> src/android/camera_hal_config.cpp | 16 +++++-----
> src/libcamera/yaml_parser.cpp | 39 +++++++++++++++++-------
> test/yaml-parser.cpp | 18 +++++------
> 4 files changed, 47 insertions(+), 30 deletions(-)
>
> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> index b4f852b1ce54..be5f0914703f 100644
> --- a/include/libcamera/internal/yaml_parser.h
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -7,7 +7,6 @@
>
> #pragma once
>
> -#include <cstdio>
> #include <map>
> #include <string>
> #include <vector>
> @@ -18,6 +17,7 @@
>
> namespace libcamera {
>
> +class File;
> class YamlParserContext;
>
> class YamlObject
> @@ -82,7 +82,7 @@ private:
> class YamlParser final
> {
> public:
> - static std::unique_ptr<YamlObject> parse(std::FILE *fh);
> + static std::unique_ptr<YamlObject> parse(File &file);
> };
>
> } /* namespace libcamera */
> diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp
> index 8ba8738cc6b6..ac484b8df1bd 100644
> --- a/src/android/camera_hal_config.cpp
> +++ b/src/android/camera_hal_config.cpp
> @@ -10,6 +10,7 @@
> #include <stdlib.h>
> #include <string>
>
> +#include <libcamera/base/file.h>
> #include <libcamera/base/log.h>
>
> #include "libcamera/internal/yaml_parser.h"
> @@ -27,7 +28,7 @@ class CameraHalConfig::Private : public Extensible::Private
> public:
> Private();
>
> - int parseConfigFile(FILE *fh, std::map<std::string, CameraConfigData> *cameras);
> + int parseConfigFile(File &file, std::map<std::string, CameraConfigData> *cameras);
>
> private:
> int parseCameraConfigData(const std::string &cameraId, const YamlObject &);
> @@ -41,7 +42,7 @@ CameraHalConfig::Private::Private()
> {
> }
>
> -int CameraHalConfig::Private::parseConfigFile(FILE *fh,
> +int CameraHalConfig::Private::parseConfigFile(File &file,
> std::map<std::string, CameraConfigData> *cameras)
> {
> /*
> @@ -65,7 +66,7 @@ int CameraHalConfig::Private::parseConfigFile(FILE *fh,
>
> cameras_ = cameras;
>
> - std::unique_ptr<YamlObject> root = YamlParser::parse(fh);
> + std::unique_ptr<YamlObject> root = YamlParser::parse(file);
> if (!root)
> return -EINVAL;
>
> @@ -169,9 +170,9 @@ int CameraHalConfig::parseConfigurationFile()
> return -ENOENT;
> }
>
> - FILE *fh = fopen(filePath.c_str(), "r");
> - if (!fh) {
> - int ret = -errno;
> + File file(filePath);
> + if (!file.open(File::OpenModeFlag::ReadOnly)) {
> + int ret = file.error();
> LOG(HALConfig, Error) << "Failed to open configuration file "
> << filePath << ": " << strerror(-ret);
> return ret;
> @@ -179,8 +180,7 @@ int CameraHalConfig::parseConfigurationFile()
>
> exists_ = true;
>
> - int ret = _d()->parseConfigFile(fh, &cameras_);
> - fclose(fh);
> + int ret = _d()->parseConfigFile(file, &cameras_);
> if (ret)
> return -EINVAL;
>
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index 5b872dbb0a2d..85f6694f5fde 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -11,6 +11,7 @@
> #include <errno.h>
> #include <functional>
>
> +#include <libcamera/base/file.h>
> #include <libcamera/base/log.h>
>
> #include <yaml.h>
> @@ -345,7 +346,7 @@ public:
> YamlParserContext();
> ~YamlParserContext();
>
> - int init(std::FILE *fh);
> + int init(File &file);
> int parseContent(YamlObject &yamlObject);
>
> private:
> @@ -358,6 +359,9 @@ private:
> };
> using EventPtr = std::unique_ptr<yaml_event_t, EventDeleter>;
>
> + static int yamlRead(void *data, unsigned char *buffer, size_t size,
> + size_t *sizeRead);
> +
> EventPtr nextEvent();
>
> void readValue(std::string &value, EventPtr event);
> @@ -399,13 +403,13 @@ YamlParserContext::~YamlParserContext()
> * \param[in] fh The YAML file to parse
> *
> * Prior to parsing the YAML content, the YamlParserContext must be initialized
> - * with an opened FILE to create an internal parser. The FILE needs to stay
> - * valid during the process.
> + * with a file to create an internal parser. The file needs to stay valid until
> + * parsing completes.
> *
> * \return 0 on success or a negative error code otherwise
> * \retval -EINVAL The parser has failed to initialize
> */
> -int YamlParserContext::init(std::FILE *fh)
> +int YamlParserContext::init(File &file)
> {
> /* yaml_parser_initialize returns 1 when it succeededs */
> if (!yaml_parser_initialize(&parser_)) {
> @@ -413,11 +417,25 @@ int YamlParserContext::init(std::FILE *fh)
> return -EINVAL;
> }
> parserValid_ = true;
> - yaml_parser_set_input_file(&parser_, fh);
> + yaml_parser_set_input(&parser_, &YamlParserContext::yamlRead, &file);
>
> return 0;
> }
>
> +int YamlParserContext::yamlRead(void *data, unsigned char *buffer, size_t size,
> + size_t *sizeRead)
> +{
> + File *file = static_cast<File *>(data);
> +
> + Span<unsigned char> buf{ buffer, size };
> + ssize_t ret = file->read(buf);
> + if (ret < 0)
> + return 0;
> +
> + *sizeRead = ret;
> + return 1;
> +}
> +
> /**
> * \fn YamlParserContext::nextEvent()
> * \brief Get the next event
> @@ -655,21 +673,20 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
> */
>
> /**
> - * \fn YamlParser::parse()
> * \brief Parse a YAML file as a YamlObject
> - * \param[in] fh The YAML file to parse
> + * \param[in] file The YAML file to parse
> *
> - * The YamlParser::parse() function takes an open FILE, parses its contents, and
> + * The YamlParser::parse() function takes a 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.
> + * document.
> *
> * \return Pointer to result YamlObject on success or nullptr otherwise
> */
> -std::unique_ptr<YamlObject> YamlParser::parse(std::FILE *fh)
> +std::unique_ptr<YamlObject> YamlParser::parse(File &file)
> {
> YamlParserContext context;
>
> - if (context.init(fh))
> + if (context.init(file))
> return nullptr;
>
> std::unique_ptr<YamlObject> root(new YamlObject());
> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> index c5b4ddbb19e5..e75f8fe8f642 100644
> --- a/test/yaml-parser.cpp
> +++ b/test/yaml-parser.cpp
> @@ -9,6 +9,8 @@
> #include <string>
> #include <unistd.h>
>
> +#include <libcamera/base/file.h>
> +
> #include "libcamera/internal/yaml_parser.h"
>
> #include "test.h"
> @@ -69,29 +71,27 @@ protected:
> int run()
> {
> /* Test invalid YAML file */
> - FILE *fh = fopen(invalidYamlFile_.c_str(), "r");
> - if (!fh) {
> + File file{ invalidYamlFile_ };
> + if (!file.open(File::OpenModeFlag::ReadOnly)) {
> cerr << "Fail to open invalid YAML file" << std::endl;
> return TestFail;
> }
>
> - std::unique_ptr<YamlObject> root = YamlParser::parse(fh);
> - fclose(fh);
> -
> + std::unique_ptr<YamlObject> root = YamlParser::parse(file);
> if (root) {
> cerr << "Invalid YAML file parse successfully" << std::endl;
> return TestFail;
> }
>
> /* Test YAML file */
> - fh = fopen(testYamlFile_.c_str(), "r");
> - if (!fh) {
> + file.close();
> + file.setFileName(testYamlFile_);
> + if (!file.open(File::OpenModeFlag::ReadOnly)) {
> cerr << "Fail to open test YAML file" << std::endl;
> return TestFail;
> }
>
> - root = YamlParser::parse(fh);
> - fclose(fh);
> + root = YamlParser::parse(file);
>
> if (!root) {
> cerr << "Fail to parse test YAML file: " << std::endl;
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list