{"id":13116,"url":"https://patchwork.libcamera.org/api/1.1/patches/13116/?format=json","web_url":"https://patchwork.libcamera.org/patch/13116/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210725171827.23643-6-laurent.pinchart@ideasonboard.com>","date":"2021-07-25T17:18:27","name":"[libcamera-devel,v2,5/5] libcamera: file: Turn MapFlag and OpenModeFlag into enum class","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"d3a32c715d2edb7f6ddf18eab7e70a9faaa91874","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/13116/mbox/","series":[{"id":2276,"url":"https://patchwork.libcamera.org/api/1.1/series/2276/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2276","date":"2021-07-25T17:18:22","name":"libcamera: Add type-safe enum-based flags","version":2,"mbox":"https://patchwork.libcamera.org/series/2276/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13116/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13116/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1600EC322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 25 Jul 2021 17:18:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92BA6687C8;\n\tSun, 25 Jul 2021 19:18:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 76067687B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 25 Jul 2021 19:18:38 +0200 (CEST)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 20713DD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 25 Jul 2021 19:18:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RBB/zA8H\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627233518;\n\tbh=zdY7+JwbHQvOH7TirKuzoqx/i1PEJdqE3iQ5heilfCE=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=RBB/zA8Hi6o7uU1AOeIkGI6RyQqUoFi7+8DewK8zAz4tCVHONi32zwh5dX0pRaS9W\n\t2lXSQ9e0d/hJ+razOLFuqeFK+qzu2maSEAwtiZ4qKxodGDfj79JXilJvSRburWjYMF\n\tnzGUBxNewDoS7HS2O8n9kC+jsvB3V/ivVys7Kso0=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Sun, 25 Jul 2021 20:18:27 +0300","Message-Id":"<20210725171827.23643-6-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.31.1","In-Reply-To":"<20210725171827.23643-1-laurent.pinchart@ideasonboard.com>","References":"<20210725171827.23643-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 5/5] libcamera: file: Turn MapFlag and\n\tOpenModeFlag into enum class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"Add type safety by turning the MapFlag and OpenModeFlag enum into enum\nclass.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/base/file.h | 13 ++++++++-----\n src/ipa/vimc/vimc.cpp         |  2 +-\n src/libcamera/base/file.cpp   | 34 +++++++++++++++++-----------------\n src/libcamera/ipa_manager.cpp |  2 +-\n src/libcamera/ipa_module.cpp  |  6 +++---\n test/file.cpp                 | 32 ++++++++++++++++----------------\n 6 files changed, 46 insertions(+), 43 deletions(-)","diff":"diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h\nindex eebb24024025..60851d385587 100644\n--- a/include/libcamera/base/file.h\n+++ b/include/libcamera/base/file.h\n@@ -23,14 +23,14 @@ namespace libcamera {\n class File\n {\n public:\n-\tenum MapFlag {\n-\t\tMapNoOption = 0,\n-\t\tMapPrivate = (1 << 0),\n+\tenum class MapFlag {\n+\t\tNoOption = 0,\n+\t\tPrivate = (1 << 0),\n \t};\n \n \tusing MapFlags = Flags<MapFlag>;\n \n-\tenum OpenModeFlag {\n+\tenum class OpenModeFlag {\n \t\tNotOpen = 0,\n \t\tReadOnly = (1 << 0),\n \t\tWriteOnly = (1 << 1),\n@@ -62,7 +62,7 @@ public:\n \tssize_t write(const Span<const uint8_t> &data);\n \n \tSpan<uint8_t> map(off_t offset = 0, ssize_t size = -1,\n-\t\t\t  MapFlags flags = MapNoOption);\n+\t\t\t  MapFlags flags = MapFlag::NoOption);\n \tbool unmap(uint8_t *addr);\n \n \tstatic bool exists(const std::string &name);\n@@ -80,6 +80,9 @@ private:\n \tstd::map<void *, size_t> maps_;\n };\n \n+LIBCAMERA_FLAGS_ENABLE_OPERATORS(File::MapFlag)\n+LIBCAMERA_FLAGS_ENABLE_OPERATORS(File::OpenModeFlag)\n+\n } /* namespace libcamera */\n \n #endif /* __LIBCAMERA_BASE_FILE_H__ */\ndiff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\nindex 2f5758539a0e..0c0ee006fdc7 100644\n--- a/src/ipa/vimc/vimc.cpp\n+++ b/src/ipa/vimc/vimc.cpp\n@@ -62,7 +62,7 @@ int IPAVimc::init(const IPASettings &settings)\n \t\t<< settings.configurationFile;\n \n \tFile conf(settings.configurationFile);\n-\tif (!conf.open(File::ReadOnly)) {\n+\tif (!conf.open(File::OpenModeFlag::ReadOnly)) {\n \t\tLOG(IPAVimc, Error) << \"Failed to open configuration file\";\n \t\treturn -EINVAL;\n \t}\ndiff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp\nindex 0860457421a1..376bf2ead382 100644\n--- a/src/libcamera/base/file.cpp\n+++ b/src/libcamera/base/file.cpp\n@@ -45,9 +45,9 @@ LOG_DEFINE_CATEGORY(File)\n /**\n  * \\enum File::MapFlag\n  * \\brief Flags for the File::map() function\n- * \\var File::MapNoOption\n+ * \\var File::MapFlag::NoOption\n  * \\brief No option (used as default value)\n- * \\var File::MapPrivate\n+ * \\var File::MapFlag::Private\n  * \\brief The memory region is mapped as private, changes are not reflected in\n  * the file constents\n  */\n@@ -60,13 +60,13 @@ LOG_DEFINE_CATEGORY(File)\n /**\n  * \\enum File::OpenModeFlag\n  * \\brief Mode in which a file is opened\n- * \\var File::NotOpen\n+ * \\var File::OpenModeFlag::NotOpen\n  * \\brief The file is not open\n- * \\var File::ReadOnly\n+ * \\var File::OpenModeFlag::ReadOnly\n  * \\brief The file is open for reading\n- * \\var File::WriteOnly\n+ * \\var File::OpenModeFlag::WriteOnly\n  * \\brief The file is open for writing\n- * \\var File::ReadWrite\n+ * \\var File::OpenModeFlag::ReadWrite\n  * \\brief The file is open for reading and writing\n  */\n \n@@ -83,7 +83,7 @@ LOG_DEFINE_CATEGORY(File)\n  * before performing I/O operations.\n  */\n File::File(const std::string &name)\n-\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n+\t: name_(name), fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0)\n {\n }\n \n@@ -94,7 +94,7 @@ File::File(const std::string &name)\n  * setFileName().\n  */\n File::File()\n-\t: fd_(-1), mode_(NotOpen), error_(0)\n+\t: fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0)\n {\n }\n \n@@ -173,8 +173,8 @@ bool File::open(File::OpenMode mode)\n \t\treturn false;\n \t}\n \n-\tint flags = static_cast<OpenMode::Type>(mode & ReadWrite) - 1;\n-\tif (mode & WriteOnly)\n+\tint flags = static_cast<OpenMode::Type>(mode & OpenModeFlag::ReadWrite) - 1;\n+\tif (mode & OpenModeFlag::WriteOnly)\n \t\tflags |= O_CREAT;\n \n \tfd_ = ::open(name_.c_str(), flags, 0666);\n@@ -214,7 +214,7 @@ void File::close()\n \n \t::close(fd_);\n \tfd_ = -1;\n-\tmode_ = NotOpen;\n+\tmode_ = OpenModeFlag::NotOpen;\n }\n \n /**\n@@ -374,8 +374,8 @@ ssize_t File::write(const Span<const uint8_t> &data)\n  * offset until the end of the file.\n  *\n  * The mapping memory protection is controlled by the file open mode, unless \\a\n- * flags contains MapPrivate in which case the region is mapped in read/write\n- * mode.\n+ * flags contains MapFlag::Private in which case the region is mapped in\n+ * read/write mode.\n  *\n  * The error() status is updated.\n  *\n@@ -398,14 +398,14 @@ Span<uint8_t> File::map(off_t offset, ssize_t size, File::MapFlags flags)\n \t\tsize -= offset;\n \t}\n \n-\tint mmapFlags = flags & MapPrivate ? MAP_PRIVATE : MAP_SHARED;\n+\tint mmapFlags = flags & MapFlag::Private ? MAP_PRIVATE : MAP_SHARED;\n \n \tint prot = 0;\n-\tif (mode_ & ReadOnly)\n+\tif (mode_ & OpenModeFlag::ReadOnly)\n \t\tprot |= PROT_READ;\n-\tif (mode_ & WriteOnly)\n+\tif (mode_ & OpenModeFlag::WriteOnly)\n \t\tprot |= PROT_WRITE;\n-\tif (flags & MapPrivate)\n+\tif (flags & MapFlag::Private)\n \t\tprot |= PROT_WRITE;\n \n \tvoid *map = mmap(NULL, size, prot, mmapFlags, fd_, offset);\ndiff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\nindex 558408969c31..771150ba0b35 100644\n--- a/src/libcamera/ipa_manager.cpp\n+++ b/src/libcamera/ipa_manager.cpp\n@@ -285,7 +285,7 @@ bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n \t}\n \n \tFile file{ ipa->path() };\n-\tif (!file.open(File::ReadOnly))\n+\tif (!file.open(File::OpenModeFlag::ReadOnly))\n \t\treturn false;\n \n \tSpan<uint8_t> data = file.map();\ndiff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\nindex adfb8d407697..7c52ad8d5796 100644\n--- a/src/libcamera/ipa_module.cpp\n+++ b/src/libcamera/ipa_module.cpp\n@@ -275,7 +275,7 @@ IPAModule::~IPAModule()\n int IPAModule::loadIPAModuleInfo()\n {\n \tFile file{ libPath_ };\n-\tif (!file.open(File::ReadOnly)) {\n+\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n \t\tLOG(IPAModule, Error) << \"Failed to open IPA library: \"\n \t\t\t\t      << strerror(-file.error());\n \t\treturn file.error();\n@@ -317,13 +317,13 @@ int IPAModule::loadIPAModuleInfo()\n \n \t/* Load the signature. Failures are not fatal. */\n \tFile sign{ libPath_ + \".sign\" };\n-\tif (!sign.open(File::ReadOnly)) {\n+\tif (!sign.open(File::OpenModeFlag::ReadOnly)) {\n \t\tLOG(IPAModule, Debug)\n \t\t\t<< \"IPA module \" << libPath_ << \" is not signed\";\n \t\treturn 0;\n \t}\n \n-\tdata = sign.map(0, -1, File::MapPrivate);\n+\tdata = sign.map(0, -1, File::MapFlag::Private);\n \tsignature_.resize(data.size());\n \tmemcpy(signature_.data(), data.data(), data.size());\n \ndiff --git a/test/file.cpp b/test/file.cpp\nindex d768e3235b8c..9ac151c944b5 100644\n--- a/test/file.cpp\n+++ b/test/file.cpp\n@@ -72,7 +72,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.openMode() != File::NotOpen) {\n+\t\tif (file.openMode() != File::OpenModeFlag::NotOpen) {\n \t\t\tcerr << \"File has invalid open mode after construction\"\n \t\t\t     << endl;\n \t\t\treturn TestFail;\n@@ -83,7 +83,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.open(File::ReadWrite)) {\n+\t\tif (file.open(File::OpenModeFlag::ReadWrite)) {\n \t\t\tcerr << \"Opening unnamed file succeeded\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -111,7 +111,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.openMode() != File::NotOpen) {\n+\t\tif (file.openMode() != File::OpenModeFlag::NotOpen) {\n \t\t\tcerr << \"Invalid file has invalid open mode after construction\"\n \t\t\t     << endl;\n \t\t\treturn TestFail;\n@@ -122,7 +122,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.open(File::ReadWrite)) {\n+\t\tif (file.open(File::OpenModeFlag::ReadWrite)) {\n \t\t\tcerr << \"Opening invalid file succeeded\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -140,7 +140,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.openMode() != File::NotOpen) {\n+\t\tif (file.openMode() != File::OpenModeFlag::NotOpen) {\n \t\t\tcerr << \"Valid file has invalid open mode after construction\"\n \t\t\t     << endl;\n \t\t\treturn TestFail;\n@@ -152,7 +152,7 @@ protected:\n \t\t}\n \n \t\t/* Test open and close. */\n-\t\tif (!file.open(File::ReadWrite)) {\n+\t\tif (!file.open(File::OpenModeFlag::ReadWrite)) {\n \t\t\tcerr << \"Opening file failed\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -162,7 +162,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.openMode() != File::ReadWrite) {\n+\t\tif (file.openMode() != File::OpenModeFlag::ReadWrite) {\n \t\t\tcerr << \"Open file has invalid open mode\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -174,7 +174,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.openMode() != File::NotOpen) {\n+\t\tif (file.openMode() != File::OpenModeFlag::NotOpen) {\n \t\t\tcerr << \"Closed file has invalid open mode\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -187,7 +187,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tfile.open(File::ReadOnly);\n+\t\tfile.open(File::OpenModeFlag::ReadOnly);\n \n \t\tssize_t size = file.size();\n \t\tif (size <= 0) {\n@@ -205,12 +205,12 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (file.open(File::ReadOnly)) {\n+\t\tif (file.open(File::OpenModeFlag::ReadOnly)) {\n \t\t\tcerr << \"Read-only open succeeded on nonexistent file\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (!file.open(File::WriteOnly)) {\n+\t\tif (!file.open(File::OpenModeFlag::WriteOnly)) {\n \t\t\tcerr << \"Write-only open failed on nonexistent file\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -238,7 +238,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tfile.open(File::ReadOnly);\n+\t\tfile.open(File::OpenModeFlag::ReadOnly);\n \n \t\tif (file.write(buffer) >= 0) {\n \t\t\tcerr << \"Write succeeded on read-only file\" << endl;\n@@ -247,7 +247,7 @@ protected:\n \n \t\tfile.close();\n \n-\t\tfile.open(File::ReadWrite);\n+\t\tfile.open(File::OpenModeFlag::ReadWrite);\n \n \t\tif (file.write({ buffer.data(), 9 }) != 9) {\n \t\t\tcerr << \"Write test failed\" << endl;\n@@ -278,7 +278,7 @@ protected:\n \n \t\t/* Test mapping and unmapping. */\n \t\tfile.setFileName(\"/proc/self/exe\");\n-\t\tfile.open(File::ReadOnly);\n+\t\tfile.open(File::OpenModeFlag::ReadOnly);\n \n \t\tSpan<uint8_t> data = file.map();\n \t\tif (data.empty()) {\n@@ -316,9 +316,9 @@ protected:\n \n \t\t/* Test private mapping. */\n \t\tfile.setFileName(fileName_);\n-\t\tfile.open(File::ReadWrite);\n+\t\tfile.open(File::OpenModeFlag::ReadWrite);\n \n-\t\tdata = file.map(0, -1, File::MapPrivate);\n+\t\tdata = file.map(0, -1, File::MapFlag::Private);\n \t\tif (data.empty()) {\n \t\t\tcerr << \"Private mapping failed\" << endl;\n \t\t\treturn TestFail;\n","prefixes":["libcamera-devel","v2","5/5"]}