From 081f4a7fcb1a0701887b5e09c0ed5f0656c71fd3 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 15 Apr 2022 15:36:06 +0100 Subject: Enable and fixup all conversion/cast warnings --- Jamroot.jam | 7 ++++++- netfs/daemon/daemonFile.cpp | 13 +++++++------ netfs/daemon/daemonVolume.cpp | 15 ++++++++------- netfs/fuse/fuseApp.h | 15 ++++++++------- netfs/fuse/fuseDirs.cpp | 4 ++-- netfs/fuse/fuseFiles.cpp | 8 ++++---- netfs/fuse/fuseMappersImpl.cpp | 9 +++++---- netfs/ice/numeric.h | 14 ++++++++++++++ netfs/ice/typeConverter.cpp | 6 +++--- netfs/lib/defaultMapper.cpp | 7 ++++--- netfs/unittests/testCore.cpp | 23 +++++++++++------------ netfs/unittests/testLib.cpp | 18 +++++++++--------- 12 files changed, 81 insertions(+), 58 deletions(-) diff --git a/Jamroot.jam b/Jamroot.jam index 8551ded..c13f68c 100644 --- a/Jamroot.jam +++ b/Jamroot.jam @@ -15,9 +15,14 @@ project hidden "-Wl,-z,defs,--warn-once,--gc-sections" release:on - debuggcc:-Wlogical-op debug:extra debug:on + debug:-Wold-style-cast + debug:-Wcast-align + debug:-Wconversion + debug:-Wsign-conversion + gcc,debug:-Wlogical-op + gcc,debug:-Wuseless-cast coverage:on tidy:boost-* tidy:bugprone-* diff --git a/netfs/daemon/daemonFile.cpp b/netfs/daemon/daemonFile.cpp index 610ee21..7b2dd8e 100644 --- a/netfs/daemon/daemonFile.cpp +++ b/netfs/daemon/daemonFile.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -45,14 +46,14 @@ NetFS::Buffer FileServer::read(Ice::Long offset, Ice::Long size, const Ice::Current &) { NetFS::Buffer buf; - buf.resize(size); + buf.resize(safe {size}); errno = 0; - auto r = pread(fd, &buf[0], size, offset); + auto r = pread(fd, &buf[0], safe {size}, offset); if (r == -1) { throw NetFS::SystemError(errno); } - else if (r != size) { - buf.resize(r); + else if (std::cmp_not_equal(r, size)) { + buf.resize(safe {r}); } return buf; } @@ -61,7 +62,7 @@ void FileServer::write(Ice::Long offset, Ice::Long size, const NetFS::Buffer data, const Ice::Current &) { errno = 0; - if (pwrite(fd, &data.front(), size, offset) != size) { + if (pwrite(fd, &data.front(), safe {size}, offset) != size) { throw NetFS::SystemError(errno); } } @@ -73,7 +74,7 @@ FileServer::copyrange(NetFS::FilePrxPtr to, Ice::Long offsrc, Ice::Long offdst, if (auto obj = ice.adapter->findByProxy(to); auto file = std::dynamic_pointer_cast(obj)) { errno = 0; off_t src = offsrc, dst = offdst; - if (auto rtn = copy_file_range(fd, &src, file->fd, &dst, size, flags); rtn != -1) { + if (auto rtn = copy_file_range(fd, &src, file->fd, &dst, safe {size}, safe {flags}); rtn != -1) { return rtn; } throw NetFS::SystemError(errno); diff --git a/netfs/daemon/daemonVolume.cpp b/netfs/daemon/daemonVolume.cpp index 9f23b38..0384015 100644 --- a/netfs/daemon/daemonVolume.cpp +++ b/netfs/daemon/daemonVolume.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -75,7 +76,7 @@ VolumeServer::mknod(const NetFS::ReqEnv re, std::string path, Ice::Int mode, Ice errno = 0; std::filesystem::path p(resolvePath(std::move(path))); mc.AssertWriteParent(p); - if (::mknod(p.c_str(), mode, dev) != 0) { + if (::mknod(p.c_str(), safe {mode}, safe {dev}) != 0) { throw NetFS::SystemError(errno); } } @@ -138,11 +139,11 @@ VolumeServer::readlink(const NetFS::ReqEnv re, std::string path, const Ice::Curr std::string buf(PATH_MAX, 0); std::filesystem::path p(resolvePath(std::move(path))); mc.AssertRead(p); - ssize_t rc = ::readlink(p.c_str(), buf.data(), PATH_MAX); - if (rc == -1) { + auto rc = ::readlink(p.c_str(), buf.data(), PATH_MAX); + if (rc < 0) { throw NetFS::SystemError(errno); } - buf.resize(rc); + buf.resize(safe {rc}); return buf; } @@ -153,7 +154,7 @@ VolumeServer::chmod(const NetFS::ReqEnv re, std::string path, Ice::Int mode, con errno = 0; std::filesystem::path p(resolvePath(std::move(path))); mc.AssertWritePerms(p); - if (::chmod(p.c_str(), mode) != 0) { + if (::chmod(p.c_str(), safe {mode}) != 0) { throw NetFS::SystemError(errno); } } @@ -165,7 +166,7 @@ VolumeServer::chown(const NetFS::ReqEnv re, std::string path, Ice::Int uid, Ice: errno = 0; std::filesystem::path p(resolvePath(std::move(path))); mc.AssertWrite(p); - if (::lchown(p.c_str(), uid, gid) != 0) { + if (::lchown(p.c_str(), safe {uid}, safe {gid}) != 0) { throw NetFS::SystemError(errno); } } @@ -282,7 +283,7 @@ VolumeServer::mkdir(const NetFS::ReqEnv re, std::string path, Ice::Int mode, con errno = 0; std::filesystem::path p(resolvePath(std::move(path))); mc.AssertWriteParent(p); - if (::mkdir(p.c_str(), mode) != 0) { + if (::mkdir(p.c_str(), safe {mode}) != 0) { throw NetFS::SystemError(errno); } if (::chown(p.c_str(), mc.myu, mc.myg) != 0) { diff --git a/netfs/fuse/fuseApp.h b/netfs/fuse/fuseApp.h index 187e2ba..5652a41 100644 --- a/netfs/fuse/fuseApp.h +++ b/netfs/fuse/fuseApp.h @@ -18,13 +18,14 @@ namespace NetFS { class DLL_PUBLIC FuseApp : public FuseAppBaseT { private: + using FuseHandleTypeId = decltype(fuse_file_info::fh); class OpenDir; using OpenDirPtr = std::shared_ptr; - using OpenDirs = std::map; + using OpenDirs = std::map; class OpenFile; using OpenFilePtr = std::shared_ptr; - using OpenFiles = std::map; + using OpenFiles = std::map; public: explicit FuseApp(Ice::StringSeq &&); @@ -85,10 +86,10 @@ namespace NetFS { static NetFS::Client::ResourcePtr configureFromUri(const std::string &); protected: - template void setProxy(uint64_t & fh, const Params &...); - template Handle getProxy(uint64_t localID); - template void clearProxy(uint64_t localID); - template std::map & getMap(); + template void setProxy(FuseHandleTypeId & fh, const Params &...); + template Handle getProxy(FuseHandleTypeId localID); + template void clearProxy(FuseHandleTypeId localID); + template std::map & getMap(); template static inline Rtn waitOnWriteRangeAndThen(size_t s, off_t o, const OpenFilePtr & of, const F & f); @@ -109,7 +110,7 @@ namespace NetFS { OpenDirs openDirs; OpenFiles openFiles; - int openHandleId {0}; + FuseHandleTypeId openHandleId {}; EntryTypeConverter converter; diff --git a/netfs/fuse/fuseDirs.cpp b/netfs/fuse/fuseDirs.cpp index bc13cd1..a4fa233 100644 --- a/netfs/fuse/fuseDirs.cpp +++ b/netfs/fuse/fuseDirs.cpp @@ -7,7 +7,7 @@ namespace NetFS { FuseApp::OpenDir::OpenDir(DirectoryPrxPtr r, std::string p) : remote(std::move(r)), path(std::move(p)) { } template<> - std::map & + std::map & FuseApp::getMap() { return openDirs; @@ -62,7 +62,7 @@ namespace NetFS { else { // Standard read dir cannot know the local system cannot represent the inode for (const auto & e : od->remote->readdir()) { - filler(buf, e.c_str(), nullptr, 0, (fuse_fill_dir_flags)0); + filler(buf, e.c_str(), nullptr, 0, fuse_fill_dir_flags {}); } } return 0; diff --git a/netfs/fuse/fuseFiles.cpp b/netfs/fuse/fuseFiles.cpp index 3d23338..1ab568c 100644 --- a/netfs/fuse/fuseFiles.cpp +++ b/netfs/fuse/fuseFiles.cpp @@ -14,7 +14,7 @@ namespace NetFS { } template<> - std::map & + std::map & FuseApp::getMap() { return openFiles; @@ -48,7 +48,7 @@ namespace NetFS { FuseApp::OpenFile::BGs::interval_type FuseApp::OpenFile::range(off_t o, size_t s) { - return OpenFile::BGs::interval_type::right_open(o, o + s); + return OpenFile::BGs::interval_type::right_open(safe {o}, safe {o} + s); } int @@ -125,7 +125,7 @@ namespace NetFS { } else { std::vector> overlap; - overlap.reserve(std::distance(R.first, R.second)); + overlap.reserve(safe {std::distance(R.first, R.second)}); for (auto i = R.first; i != R.second; i++) { overlap.push_back(i->second); } @@ -175,7 +175,7 @@ namespace NetFS { waitOnWriteRangeAndThen(s, o, of, [o, s, buf, &of, remote](const auto & key) { auto p = std::make_shared(); remote->writeAsync( - o, s, Buffer(buf, buf + s), + o, safe {s}, Buffer(buf, buf + s), [p, of, key]() { p->promise.set_value(); ScopeLock(of->_lock) { diff --git a/netfs/fuse/fuseMappersImpl.cpp b/netfs/fuse/fuseMappersImpl.cpp index c7fef9e..7e129f6 100644 --- a/netfs/fuse/fuseMappersImpl.cpp +++ b/netfs/fuse/fuseMappersImpl.cpp @@ -1,5 +1,6 @@ #include "fuseMappersImpl.h" #include +#include namespace NetFS::Client { constexpr int MASK_EVERYTHING = ~0; @@ -19,8 +20,8 @@ namespace NetFS::Client { Mapping::Transport HideUnknownMapperImpl::mapFileSystem(int uid, int gid) { - auto u = users->getEntry(uid); - auto g = groups->getEntry(gid); + auto u = users->getEntry(safe {uid}); + auto g = groups->getEntry(safe {gid}); if (!u || !g) { throw NetFS::SystemError(EPERM); } @@ -50,8 +51,8 @@ namespace NetFS::Client { Mapping::Transport MaskUnknownMapperImpl::mapFileSystem(int uid, int gid) { - auto u = users->getEntry(uid); - auto g = groups->getEntry(gid); + auto u = users->getEntry(safe {uid}); + auto g = groups->getEntry(safe {gid}); if (!u || !g) { throw NetFS::SystemError(EPERM); } diff --git a/netfs/ice/numeric.h b/netfs/ice/numeric.h index 3b49f4b..cdfec2a 100644 --- a/netfs/ice/numeric.h +++ b/netfs/ice/numeric.h @@ -48,4 +48,18 @@ private: constexpr static T max {std::numeric_limits::max()}; }; +template +auto +operator+(const Left left, const safe right) +{ + return left + static_cast(right); +} + +template +auto +operator+(const safe left, const Right right) +{ + return static_cast(left) + right; +} + #endif diff --git a/netfs/ice/typeConverter.cpp b/netfs/ice/typeConverter.cpp index a0c8437..896d078 100644 --- a/netfs/ice/typeConverter.cpp +++ b/netfs/ice/typeConverter.cpp @@ -12,8 +12,8 @@ EntryTypeConverter::convert(const NetFS::Attr & a) const s.st_ino = safe {a.inode}; s.st_mode = safe {a.mode & ~map.mask}; s.st_nlink = safe {a.links}; - s.st_uid = map.uid; - s.st_gid = map.gid; + s.st_uid = safe {map.uid}; + s.st_gid = safe {map.gid}; s.st_rdev = safe {a.rdev}; s.st_size = safe {a.size}; s.st_blksize = safe {a.blockSize}; @@ -51,7 +51,7 @@ EntryTypeConverter::convert(const struct stat & s) const NetFS::Attr a {}; a.dev = safe {s.st_dev}; a.inode = safe {s.st_ino}; - a.mode = safe {s.st_mode & ~map.mask}; + a.mode = safe {s.st_mode & static_cast(~map.mask)}; a.links = safe {s.st_nlink}; a.uid = std::move(map.username); a.gid = std::move(map.groupname); diff --git a/netfs/lib/defaultMapper.cpp b/netfs/lib/defaultMapper.cpp index c57f340..1845933 100644 --- a/netfs/lib/defaultMapper.cpp +++ b/netfs/lib/defaultMapper.cpp @@ -1,5 +1,6 @@ #include "defaultMapper.h" #include +#include namespace NetFS::Mapping { FileSystem @@ -10,14 +11,14 @@ namespace NetFS::Mapping { if (!u || !g) { throw NetFS::SystemError(EPERM); } - return {static_cast(u->id), static_cast(g->id), 0}; + return {safe {u->id}, safe {g->id}, 0}; } Transport DefaultMapper::mapFileSystem(int uid, int gid) { - auto u = users->getEntry(uid); - auto g = groups->getEntry(gid); + auto u = users->getEntry(safe {uid}); + auto g = groups->getEntry(safe {gid}); if (!u || !g) { throw NetFS::SystemError(EPERM); } diff --git a/netfs/unittests/testCore.cpp b/netfs/unittests/testCore.cpp index c99999f..f472607 100644 --- a/netfs/unittests/testCore.cpp +++ b/netfs/unittests/testCore.cpp @@ -316,16 +316,16 @@ BOOST_AUTO_TEST_CASE(bgwriteOverlapped, *boost::unit_test::timeout(2)) struct stat st { }; fi.flags = O_RDWR; constexpr auto s = sizeof(int32_t); - auto N = 20; + constexpr auto N = 20U; BOOST_REQUIRE_EQUAL(fuse->create("/test", 0600, &fi), 0); - for (int32_t n = 0; n < N; n += 1) { - BOOST_REQUIRE_EQUAL(fuse->write("/test", (const char *)(&n), s, n, &fi), s); + for (auto n = 0U; n < N; n += 1) { + BOOST_REQUIRE_EQUAL(fuse->write("/test", reinterpret_cast(&n), s, n, &fi), s); } BOOST_REQUIRE_EQUAL(fuse->getattr("/test", &st, &fi), 0); BOOST_REQUIRE_EQUAL(st.st_size, N + s - 1); std::array buf; BOOST_REQUIRE_EQUAL(fuse->read("/test", buf.data(), s, (N - 1), &fi), s); - BOOST_REQUIRE_EQUAL(*(int *)buf.data(), N - 1); + BOOST_REQUIRE_EQUAL(*reinterpret_cast(buf.data()), N - 1); BOOST_REQUIRE_EQUAL(fuse->release("/test", &fi), 0); BOOST_REQUIRE_EQUAL(fuse->getattr("/test", &st, nullptr), 0); BOOST_REQUIRE_EQUAL(st.st_size, N + s - 1); @@ -337,16 +337,16 @@ BOOST_AUTO_TEST_CASE(bgwriteDistinct, *boost::unit_test::timeout(2)) struct stat st { }; fi.flags = O_RDWR; constexpr auto s = sizeof(int32_t); - auto N = 20; + constexpr auto N = 20U; BOOST_REQUIRE_EQUAL(fuse->create("/test", 0600, &fi), 0); - for (int32_t n = 0; n < N; n += 1) { - BOOST_REQUIRE_EQUAL(fuse->write("/test", (const char *)(&n), s, n * s, &fi), s); + for (auto n = 0U; n < N; n += 1) { + BOOST_REQUIRE_EQUAL(fuse->write("/test", reinterpret_cast(&n), s, n * s, &fi), s); } BOOST_REQUIRE_EQUAL(fuse->getattr("/test", &st, &fi), 0); BOOST_REQUIRE_EQUAL(st.st_size, N * s); std::array buf; BOOST_REQUIRE_EQUAL(fuse->read("/test", buf.data(), s, (N - 1) * s, &fi), s); - BOOST_REQUIRE_EQUAL(*(int *)buf.data(), N - 1); + BOOST_REQUIRE_EQUAL(*reinterpret_cast(buf.data()), N - 1); BOOST_REQUIRE_EQUAL(fuse->release("/test", &fi), 0); BOOST_REQUIRE_EQUAL(fuse->getattr("/test", &st, nullptr), 0); BOOST_REQUIRE_EQUAL(st.st_size, N * s); @@ -487,7 +487,6 @@ BOOST_AUTO_TEST_CASE(chown) BOOST_REQUIRE_EQUAL(fuse->getattr("/dir", &st, nullptr), 0); BOOST_REQUIRE_EQUAL(st.st_uid, getuid()); BOOST_REQUIRE_EQUAL(st.st_gid, getgid()); - BOOST_REQUIRE_EQUAL(fuse->chown("/dir", -2, getgid(), nullptr), -ENOSYS); BOOST_REQUIRE_EQUAL(fuse->chown("/dir", 99999999, getgid(), nullptr), -EPERM); BOOST_REQUIRE_EQUAL(fuse->rmdir("/dir"), 0); } @@ -532,7 +531,7 @@ BOOST_AUTO_TEST_CASE(noListDir) BOOST_REQUIRE_EQUAL(fuse->opendir("/test", &fi), 0); NetFS::NameList nl; - BOOST_REQUIRE_EQUAL(fuse->readdir("/test", &nl, &nameListAdd, 0, &fi, (fuse_readdir_flags)0), 0); + BOOST_REQUIRE_EQUAL(fuse->readdir("/test", &nl, &nameListAdd, 0, &fi, fuse_readdir_flags {}), 0); BOOST_REQUIRE_EQUAL(nl.size(), 2); std::sort(nl.begin(), nl.end()); BOOST_REQUIRE_EQUAL(nl[0], "."); @@ -542,7 +541,7 @@ BOOST_AUTO_TEST_CASE(noListDir) BOOST_REQUIRE_EQUAL(fuse->mkdir("/test/sub", 0700), 0); BOOST_REQUIRE_EQUAL(fuse->opendir("/test", &fi), 0); - BOOST_REQUIRE_EQUAL(fuse->readdir("/test", &nl, &nameListAdd, 0, &fi, (fuse_readdir_flags)0), 0); + BOOST_REQUIRE_EQUAL(fuse->readdir("/test", &nl, &nameListAdd, 0, &fi, fuse_readdir_flags {}), 0); BOOST_REQUIRE_EQUAL(nl.size(), 3); std::sort(nl.begin(), nl.end()); BOOST_REQUIRE_EQUAL(nl[0], "."); @@ -551,7 +550,7 @@ BOOST_AUTO_TEST_CASE(noListDir) BOOST_REQUIRE_EQUAL(fuse->releasedir("/test", &fi), 0); nl.clear(); - BOOST_REQUIRE_EQUAL(fuse->readdir("/test", &nl, &nameListAdd, 0, &fi, (fuse_readdir_flags)0), -EBADF); + BOOST_REQUIRE_EQUAL(fuse->readdir("/test", &nl, &nameListAdd, 0, &fi, fuse_readdir_flags {}), -EBADF); BOOST_REQUIRE_EQUAL(fuse->releasedir("/test", &fi), -EBADF); BOOST_REQUIRE_EQUAL(fuse->rmdir("/test"), -ENOTEMPTY); diff --git a/netfs/unittests/testLib.cpp b/netfs/unittests/testLib.cpp index 9ff3c5e..b6d41f2 100644 --- a/netfs/unittests/testLib.cpp +++ b/netfs/unittests/testLib.cpp @@ -123,9 +123,9 @@ BOOST_AUTO_TEST_CASE(bad_maptransport) BOOST_AUTO_TEST_CASE(bad_mapfilesystem) { - BOOST_REQUIRE_THROW(mapFileSystem(0, -1), NetFS::SystemError); - BOOST_REQUIRE_THROW(mapFileSystem(-1, 0), NetFS::SystemError); - BOOST_REQUIRE_THROW(mapFileSystem(-1, -1), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(0, 99999999), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(99999999, 0), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(99999999, 99999999), NetFS::SystemError); } BOOST_AUTO_TEST_SUITE_END(); @@ -156,9 +156,9 @@ BOOST_AUTO_TEST_CASE(bad_maptransport) BOOST_AUTO_TEST_CASE(bad_mapfilesystem) { - BOOST_REQUIRE_THROW(mapFileSystem(0, -1), NetFS::SystemError); - BOOST_REQUIRE_THROW(mapFileSystem(-1, 0), NetFS::SystemError); - BOOST_REQUIRE_THROW(mapFileSystem(-1, -1), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(0, 99999999), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(99999999, 0), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(99999999, 99999999), NetFS::SystemError); } BOOST_AUTO_TEST_SUITE_END(); @@ -198,9 +198,9 @@ BOOST_AUTO_TEST_CASE(bad_maptransport_badfallback) BOOST_AUTO_TEST_CASE(bad_mapfilesystem) { - BOOST_REQUIRE_THROW(mapFileSystem(0, -1), NetFS::SystemError); - BOOST_REQUIRE_THROW(mapFileSystem(-1, 0), NetFS::SystemError); - BOOST_REQUIRE_THROW(mapFileSystem(-1, -1), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(0, 99999999), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(99999999, 0), NetFS::SystemError); + BOOST_REQUIRE_THROW(mapFileSystem(99999999, 99999999), NetFS::SystemError); } BOOST_AUTO_TEST_SUITE_END(); -- cgit v1.2.3