[libcamera-devel,RFC,03/12] libcamera: yaml_parser: Switch from FILE to File
diff mbox series

Message ID 20220524225816.6830-4-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Replace boost JSON parser with libyaml in Raspberry Pi IPA
Related show

Commit Message

Laurent Pinchart May 24, 2022, 10:58 p.m. UTC
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@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                     | 17 +++++------
 4 files changed, 46 insertions(+), 30 deletions(-)

Patch
diff mbox series

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 28072cb86285..42f3e3aeed04 100644
--- a/src/android/camera_hal_config.cpp
+++ b/src/android/camera_hal_config.cpp
@@ -12,6 +12,7 @@ 
 
 #include <hardware/camera3.h>
 
+#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 585b545b71c5..959a657478fa 100644
--- a/test/yaml-parser.cpp
+++ b/test/yaml-parser.cpp
@@ -9,6 +9,7 @@ 
 #include <string>
 #include <unistd.h>
 
+#include <libcamera/base/file.h>
 #include <libcamera/internal/yaml_parser.h>
 
 #include "test.h"
@@ -69,29 +70,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;