| Message ID | 20210725171827.23643-6-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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@jmondi.org> Thanks j > if (data.empty()) { > cerr << "Private mapping failed" << endl; > return TestFail; > -- > Regards, > > Laurent Pinchart >
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;
Add type safety by turning the MapFlag and OpenModeFlag enum into enum class. Signed-off-by: Laurent Pinchart <laurent.pinchart@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(-)