[libcamera-devel] [PATCH v2 5/5] libcamera: file: Turn MapFlag and OpenModeFlag into enum class

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sun Jul 25 19:18:27 CEST 2021


Add type safety by turning the MapFlag and OpenModeFlag enum into enum
class.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 include/libcamera/base/file.h | 13 ++++++++-----
 src/ipa/vimc/vimc.cpp         |  2 +-
 src/libcamera/base/file.cpp   | 34 +++++++++++++++++-----------------
 src/libcamera/ipa_manager.cpp |  2 +-
 src/libcamera/ipa_module.cpp  |  6 +++---
 test/file.cpp                 | 32 ++++++++++++++++----------------
 6 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h
index eebb24024025..60851d385587 100644
--- a/include/libcamera/base/file.h
+++ b/include/libcamera/base/file.h
@@ -23,14 +23,14 @@ namespace libcamera {
 class File
 {
 public:
-	enum MapFlag {
-		MapNoOption = 0,
-		MapPrivate = (1 << 0),
+	enum class MapFlag {
+		NoOption = 0,
+		Private = (1 << 0),
 	};
 
 	using MapFlags = Flags<MapFlag>;
 
-	enum OpenModeFlag {
+	enum class OpenModeFlag {
 		NotOpen = 0,
 		ReadOnly = (1 << 0),
 		WriteOnly = (1 << 1),
@@ -62,7 +62,7 @@ public:
 	ssize_t write(const Span<const uint8_t> &data);
 
 	Span<uint8_t> map(off_t offset = 0, ssize_t size = -1,
-			  MapFlags flags = MapNoOption);
+			  MapFlags flags = MapFlag::NoOption);
 	bool unmap(uint8_t *addr);
 
 	static bool exists(const std::string &name);
@@ -80,6 +80,9 @@ private:
 	std::map<void *, size_t> maps_;
 };
 
+LIBCAMERA_FLAGS_ENABLE_OPERATORS(File::MapFlag)
+LIBCAMERA_FLAGS_ENABLE_OPERATORS(File::OpenModeFlag)
+
 } /* namespace libcamera */
 
 #endif /* __LIBCAMERA_BASE_FILE_H__ */
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index 2f5758539a0e..0c0ee006fdc7 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -62,7 +62,7 @@ int IPAVimc::init(const IPASettings &settings)
 		<< settings.configurationFile;
 
 	File conf(settings.configurationFile);
-	if (!conf.open(File::ReadOnly)) {
+	if (!conf.open(File::OpenModeFlag::ReadOnly)) {
 		LOG(IPAVimc, Error) << "Failed to open configuration file";
 		return -EINVAL;
 	}
diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp
index 0860457421a1..376bf2ead382 100644
--- a/src/libcamera/base/file.cpp
+++ b/src/libcamera/base/file.cpp
@@ -45,9 +45,9 @@ LOG_DEFINE_CATEGORY(File)
 /**
  * \enum File::MapFlag
  * \brief Flags for the File::map() function
- * \var File::MapNoOption
+ * \var File::MapFlag::NoOption
  * \brief No option (used as default value)
- * \var File::MapPrivate
+ * \var File::MapFlag::Private
  * \brief The memory region is mapped as private, changes are not reflected in
  * the file constents
  */
@@ -60,13 +60,13 @@ LOG_DEFINE_CATEGORY(File)
 /**
  * \enum File::OpenModeFlag
  * \brief Mode in which a file is opened
- * \var File::NotOpen
+ * \var File::OpenModeFlag::NotOpen
  * \brief The file is not open
- * \var File::ReadOnly
+ * \var File::OpenModeFlag::ReadOnly
  * \brief The file is open for reading
- * \var File::WriteOnly
+ * \var File::OpenModeFlag::WriteOnly
  * \brief The file is open for writing
- * \var File::ReadWrite
+ * \var File::OpenModeFlag::ReadWrite
  * \brief The file is open for reading and writing
  */
 
@@ -83,7 +83,7 @@ LOG_DEFINE_CATEGORY(File)
  * before performing I/O operations.
  */
 File::File(const std::string &name)
-	: name_(name), fd_(-1), mode_(NotOpen), error_(0)
+	: name_(name), fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0)
 {
 }
 
@@ -94,7 +94,7 @@ File::File(const std::string &name)
  * setFileName().
  */
 File::File()
-	: fd_(-1), mode_(NotOpen), error_(0)
+	: fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0)
 {
 }
 
@@ -173,8 +173,8 @@ bool File::open(File::OpenMode mode)
 		return false;
 	}
 
-	int flags = static_cast<OpenMode::Type>(mode & ReadWrite) - 1;
-	if (mode & WriteOnly)
+	int flags = static_cast<OpenMode::Type>(mode & OpenModeFlag::ReadWrite) - 1;
+	if (mode & OpenModeFlag::WriteOnly)
 		flags |= O_CREAT;
 
 	fd_ = ::open(name_.c_str(), flags, 0666);
@@ -214,7 +214,7 @@ void File::close()
 
 	::close(fd_);
 	fd_ = -1;
-	mode_ = NotOpen;
+	mode_ = OpenModeFlag::NotOpen;
 }
 
 /**
@@ -374,8 +374,8 @@ ssize_t File::write(const Span<const uint8_t> &data)
  * offset until the end of the file.
  *
  * The mapping memory protection is controlled by the file open mode, unless \a
- * flags contains MapPrivate in which case the region is mapped in read/write
- * mode.
+ * flags contains MapFlag::Private in which case the region is mapped in
+ * read/write mode.
  *
  * The error() status is updated.
  *
@@ -398,14 +398,14 @@ Span<uint8_t> File::map(off_t offset, ssize_t size, File::MapFlags flags)
 		size -= offset;
 	}
 
-	int mmapFlags = flags & MapPrivate ? MAP_PRIVATE : MAP_SHARED;
+	int mmapFlags = flags & MapFlag::Private ? MAP_PRIVATE : MAP_SHARED;
 
 	int prot = 0;
-	if (mode_ & ReadOnly)
+	if (mode_ & OpenModeFlag::ReadOnly)
 		prot |= PROT_READ;
-	if (mode_ & WriteOnly)
+	if (mode_ & OpenModeFlag::WriteOnly)
 		prot |= PROT_WRITE;
-	if (flags & MapPrivate)
+	if (flags & MapFlag::Private)
 		prot |= PROT_WRITE;
 
 	void *map = mmap(NULL, size, prot, mmapFlags, fd_, offset);
diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index 558408969c31..771150ba0b35 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -285,7 +285,7 @@ bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const
 	}
 
 	File file{ ipa->path() };
-	if (!file.open(File::ReadOnly))
+	if (!file.open(File::OpenModeFlag::ReadOnly))
 		return false;
 
 	Span<uint8_t> data = file.map();
diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
index adfb8d407697..7c52ad8d5796 100644
--- a/src/libcamera/ipa_module.cpp
+++ b/src/libcamera/ipa_module.cpp
@@ -275,7 +275,7 @@ IPAModule::~IPAModule()
 int IPAModule::loadIPAModuleInfo()
 {
 	File file{ libPath_ };
-	if (!file.open(File::ReadOnly)) {
+	if (!file.open(File::OpenModeFlag::ReadOnly)) {
 		LOG(IPAModule, Error) << "Failed to open IPA library: "
 				      << strerror(-file.error());
 		return file.error();
@@ -317,13 +317,13 @@ int IPAModule::loadIPAModuleInfo()
 
 	/* Load the signature. Failures are not fatal. */
 	File sign{ libPath_ + ".sign" };
-	if (!sign.open(File::ReadOnly)) {
+	if (!sign.open(File::OpenModeFlag::ReadOnly)) {
 		LOG(IPAModule, Debug)
 			<< "IPA module " << libPath_ << " is not signed";
 		return 0;
 	}
 
-	data = sign.map(0, -1, File::MapPrivate);
+	data = sign.map(0, -1, File::MapFlag::Private);
 	signature_.resize(data.size());
 	memcpy(signature_.data(), data.data(), data.size());
 
diff --git a/test/file.cpp b/test/file.cpp
index d768e3235b8c..9ac151c944b5 100644
--- a/test/file.cpp
+++ b/test/file.cpp
@@ -72,7 +72,7 @@ protected:
 			return TestFail;
 		}
 
-		if (file.openMode() != File::NotOpen) {
+		if (file.openMode() != File::OpenModeFlag::NotOpen) {
 			cerr << "File has invalid open mode after construction"
 			     << endl;
 			return TestFail;
@@ -83,7 +83,7 @@ protected:
 			return TestFail;
 		}
 
-		if (file.open(File::ReadWrite)) {
+		if (file.open(File::OpenModeFlag::ReadWrite)) {
 			cerr << "Opening unnamed file succeeded" << endl;
 			return TestFail;
 		}
@@ -111,7 +111,7 @@ protected:
 			return TestFail;
 		}
 
-		if (file.openMode() != File::NotOpen) {
+		if (file.openMode() != File::OpenModeFlag::NotOpen) {
 			cerr << "Invalid file has invalid open mode after construction"
 			     << endl;
 			return TestFail;
@@ -122,7 +122,7 @@ protected:
 			return TestFail;
 		}
 
-		if (file.open(File::ReadWrite)) {
+		if (file.open(File::OpenModeFlag::ReadWrite)) {
 			cerr << "Opening invalid file succeeded" << endl;
 			return TestFail;
 		}
@@ -140,7 +140,7 @@ protected:
 			return TestFail;
 		}
 
-		if (file.openMode() != File::NotOpen) {
+		if (file.openMode() != File::OpenModeFlag::NotOpen) {
 			cerr << "Valid file has invalid open mode after construction"
 			     << endl;
 			return TestFail;
@@ -152,7 +152,7 @@ protected:
 		}
 
 		/* Test open and close. */
-		if (!file.open(File::ReadWrite)) {
+		if (!file.open(File::OpenModeFlag::ReadWrite)) {
 			cerr << "Opening file failed" << endl;
 			return TestFail;
 		}
@@ -162,7 +162,7 @@ protected:
 			return TestFail;
 		}
 
-		if (file.openMode() != File::ReadWrite) {
+		if (file.openMode() != File::OpenModeFlag::ReadWrite) {
 			cerr << "Open file has invalid open mode" << endl;
 			return TestFail;
 		}
@@ -174,7 +174,7 @@ protected:
 			return TestFail;
 		}
 
-		if (file.openMode() != File::NotOpen) {
+		if (file.openMode() != File::OpenModeFlag::NotOpen) {
 			cerr << "Closed file has invalid open mode" << endl;
 			return TestFail;
 		}
@@ -187,7 +187,7 @@ protected:
 			return TestFail;
 		}
 
-		file.open(File::ReadOnly);
+		file.open(File::OpenModeFlag::ReadOnly);
 
 		ssize_t size = file.size();
 		if (size <= 0) {
@@ -205,12 +205,12 @@ protected:
 			return TestFail;
 		}
 
-		if (file.open(File::ReadOnly)) {
+		if (file.open(File::OpenModeFlag::ReadOnly)) {
 			cerr << "Read-only open succeeded on nonexistent file" << endl;
 			return TestFail;
 		}
 
-		if (!file.open(File::WriteOnly)) {
+		if (!file.open(File::OpenModeFlag::WriteOnly)) {
 			cerr << "Write-only open failed on nonexistent file" << endl;
 			return TestFail;
 		}
@@ -238,7 +238,7 @@ protected:
 			return TestFail;
 		}
 
-		file.open(File::ReadOnly);
+		file.open(File::OpenModeFlag::ReadOnly);
 
 		if (file.write(buffer) >= 0) {
 			cerr << "Write succeeded on read-only file" << endl;
@@ -247,7 +247,7 @@ protected:
 
 		file.close();
 
-		file.open(File::ReadWrite);
+		file.open(File::OpenModeFlag::ReadWrite);
 
 		if (file.write({ buffer.data(), 9 }) != 9) {
 			cerr << "Write test failed" << endl;
@@ -278,7 +278,7 @@ protected:
 
 		/* Test mapping and unmapping. */
 		file.setFileName("/proc/self/exe");
-		file.open(File::ReadOnly);
+		file.open(File::OpenModeFlag::ReadOnly);
 
 		Span<uint8_t> data = file.map();
 		if (data.empty()) {
@@ -316,9 +316,9 @@ protected:
 
 		/* Test private mapping. */
 		file.setFileName(fileName_);
-		file.open(File::ReadWrite);
+		file.open(File::OpenModeFlag::ReadWrite);
 
-		data = file.map(0, -1, File::MapPrivate);
+		data = file.map(0, -1, File::MapFlag::Private);
 		if (data.empty()) {
 			cerr << "Private mapping failed" << endl;
 			return TestFail;
-- 
Regards,

Laurent Pinchart



More information about the libcamera-devel mailing list