[libcamera-devel] [PATCH v2 5/5] libcamera: file: Turn MapFlag and OpenModeFlag into enum class
Jacopo Mondi
jacopo at jmondi.org
Mon Jul 26 11:55:12 CEST 2021
On Sun, Jul 25, 2021 at 08:18:27PM +0300, Laurent Pinchart wrote:
> Add type safety by turning the MapFlag and OpenModeFlag enum into enum
> class.
Ah
>
> 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);
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> if (data.empty()) {
> cerr << "Private mapping failed" << endl;
> return TestFail;
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list