From 5290947a98ae03b7e655f17cc43cbb59883c393c Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Wed, 22 Jul 2020 10:32:43 -0700 Subject: [PATCH] Update CI to clang-tidy 9 and fix warnings --- .circleci/config.yml | 9 ++++----- .clang-tidy | 1 + run-clang-tidy.sh | 2 +- src/cpp-utils/crypto/kdf/SCryptParameters.h | 4 ++++ src/cpp-utils/either.h | 9 +++++++++ src/cpp-utils/value_type/ValueType.h | 4 ++-- src/cryfs-cli/VersionChecker.cpp | 1 + .../impl/filesystem/cachingfsblobstore/DirBlobRef.h | 4 ++-- .../impl/filesystem/cachingfsblobstore/FileBlobRef.h | 4 ++-- .../impl/filesystem/cachingfsblobstore/SymlinkBlobRef.h | 4 ++-- .../caching/cache/testutils/CopyableMovableValueType.h | 1 + 11 files changed, 29 insertions(+), 14 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bd133eff..cbec79bd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -89,8 +89,7 @@ references: # They aren't set automatically unfortunately sudo ln -s /usr/bin/$CC /usr/bin/clang sudo ln -s /usr/bin/$CXX /usr/bin/clang++ - sudo ln -s /usr/bin/clang-tidy-8 /usr/bin/clang-tidy - sudo ln -s /usr/bin/run-clang-tidy-8.py /usr/bin/run-clang-tidy.py + sudo ln -s /usr/bin/clang-tidy-9 /usr/bin/clang-tidy fi # Setup build cache @@ -652,10 +651,10 @@ jobs: - store_artifacts: path: /tmp/clang-tidy-fixes environment: - CC: clang-8 - CXX: clang++-8 + CC: clang-9 + CXX: clang++-9 BUILD_TOOLSET: clang - APT_COMPILER_PACKAGE: "clang-8 clang-tidy-8" + APT_COMPILER_PACKAGE: "clang-9 clang-tidy-9" workflows: version: 2 diff --git a/.clang-tidy b/.clang-tidy index b9569c1d..2aa0d1e4 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -2,6 +2,7 @@ # TODO Enable (some of) the explicitly disabled checks. Possibly needs helper types from gsl library or similar to enable full cppcoreguidelines. # TODO Enable more checks (google-*, hicpp-*, llvm-*, modernize-*, mpi-*, performance-*, readability-*) # TODO Maybe just enable * and disable a list instead? +# TODO Check if there's new checks in clang-tidy-9 and potentially enable them Checks: | clang-diagnostic-*, clang-analyzer-*, diff --git a/run-clang-tidy.sh b/run-clang-tidy.sh index 0b0c5508..13676880 100755 --- a/run-clang-tidy.sh +++ b/run-clang-tidy.sh @@ -19,4 +19,4 @@ cat compile_commands.json|jq "map(select(.file | test(\"^$(realpath ${0%/*})/(sr rm compile_commands.json mv compile_commands2.json compile_commands.json -run-clang-tidy.py -j${NUMCORES} -quiet -header-filter "$(realpath ${0%/*})/(src|test)/.*" $@ +run-clang-tidy-9.py -j${NUMCORES} -quiet -header-filter "$(realpath ${0%/*})/(src|test)/.*" $@ diff --git a/src/cpp-utils/crypto/kdf/SCryptParameters.h b/src/cpp-utils/crypto/kdf/SCryptParameters.h index 44b7de0a..aa22dfb6 100644 --- a/src/cpp-utils/crypto/kdf/SCryptParameters.h +++ b/src/cpp-utils/crypto/kdf/SCryptParameters.h @@ -26,6 +26,10 @@ namespace cpputils { SCryptParameters(SCryptParameters &&rhs) = default; SCryptParameters &operator=(const SCryptParameters &rhs) { + if (this == &rhs) { + return *this; + } + _salt = rhs._salt.copy(); _N = rhs._N; _r = rhs._r; diff --git a/src/cpp-utils/either.h b/src/cpp-utils/either.h index 3eb5a612..511d41d2 100644 --- a/src/cpp-utils/either.h +++ b/src/cpp-utils/either.h @@ -47,7 +47,12 @@ namespace cpputils { } //TODO Try allowing copy-assignment when Left/Right types are std::is_convertible + // NOLINTNEXTLINE(cert-oop54-cpp) either &operator=(const either &rhs) noexcept(noexcept(std::declval>()._construct_left(rhs._left)) && noexcept(std::declval>()._construct_right(rhs._right))) { + if (this == &rhs) { + return *this; + } + _destruct(); _side = rhs._side; if (_side == Side::left) { @@ -59,6 +64,10 @@ namespace cpputils { } either &operator=(either &&rhs) noexcept(noexcept(std::declval>()._construct_left(std::move(rhs._left))) && noexcept(std::declval>()._construct_right(std::move(rhs._right)))) { + if (this == &rhs) { + return *this; + } + _destruct(); _side = rhs._side; if (_side == Side::left) { diff --git a/src/cpp-utils/value_type/ValueType.h b/src/cpp-utils/value_type/ValueType.h index 031cc00a..ead954ee 100644 --- a/src/cpp-utils/value_type/ValueType.h +++ b/src/cpp-utils/value_type/ValueType.h @@ -70,8 +70,8 @@ public: return *this; } constexpr IdValueType& operator=(const IdValueType& rhs) noexcept(noexcept(*std::declval() = rhs.value_)) { - value_ = rhs.value_; - return *this; + // NOLINTNEXTLINE(cppcoreguidelines-c-copy-assignment-signature,misc-unconventional-assign-operator) + return operator=(IdValueType(rhs)); } protected: diff --git a/src/cryfs-cli/VersionChecker.cpp b/src/cryfs-cli/VersionChecker.cpp index ddfbc2d8..8141bdd3 100644 --- a/src/cryfs-cli/VersionChecker.cpp +++ b/src/cryfs-cli/VersionChecker.cpp @@ -37,6 +37,7 @@ namespace cryfs_cli { if (warnings == none) { return none; } + // NOLINTNEXTLINE(bugprone-branch-clone) BOOST_FOREACH(const ptree::value_type &v, *warnings) { if(v.first == version) { return v.second.get_value(); diff --git a/src/cryfs/impl/filesystem/cachingfsblobstore/DirBlobRef.h b/src/cryfs/impl/filesystem/cachingfsblobstore/DirBlobRef.h index 3edd0327..e4db0e0b 100644 --- a/src/cryfs/impl/filesystem/cachingfsblobstore/DirBlobRef.h +++ b/src/cryfs/impl/filesystem/cachingfsblobstore/DirBlobRef.h @@ -98,11 +98,11 @@ public: return _base->AppendChildrenTo(result); } - const blockstore::BlockId &blockId() const { + const blockstore::BlockId &blockId() const override { return _base->blockId(); } - fspp::num_bytes_t lstat_size() const { + fspp::num_bytes_t lstat_size() const override { return _base->lstat_size(); } diff --git a/src/cryfs/impl/filesystem/cachingfsblobstore/FileBlobRef.h b/src/cryfs/impl/filesystem/cachingfsblobstore/FileBlobRef.h index 1cd1ef64..17bd3c67 100644 --- a/src/cryfs/impl/filesystem/cachingfsblobstore/FileBlobRef.h +++ b/src/cryfs/impl/filesystem/cachingfsblobstore/FileBlobRef.h @@ -36,11 +36,11 @@ public: return _base->flush(); } - const blockstore::BlockId &blockId() const { + const blockstore::BlockId &blockId() const override { return _base->blockId(); } - fspp::num_bytes_t lstat_size() const { + fspp::num_bytes_t lstat_size() const override { return _base->lstat_size(); } diff --git a/src/cryfs/impl/filesystem/cachingfsblobstore/SymlinkBlobRef.h b/src/cryfs/impl/filesystem/cachingfsblobstore/SymlinkBlobRef.h index 4bf1f67e..dbc0c7f8 100644 --- a/src/cryfs/impl/filesystem/cachingfsblobstore/SymlinkBlobRef.h +++ b/src/cryfs/impl/filesystem/cachingfsblobstore/SymlinkBlobRef.h @@ -20,11 +20,11 @@ public: return _base->target(); } - const blockstore::BlockId &blockId() const { + const blockstore::BlockId &blockId() const override { return _base->blockId(); } - fspp::num_bytes_t lstat_size() const { + fspp::num_bytes_t lstat_size() const override { return _base->lstat_size(); } diff --git a/test/blockstore/implementations/caching/cache/testutils/CopyableMovableValueType.h b/test/blockstore/implementations/caching/cache/testutils/CopyableMovableValueType.h index 1e241278..c85872ef 100644 --- a/test/blockstore/implementations/caching/cache/testutils/CopyableMovableValueType.h +++ b/test/blockstore/implementations/caching/cache/testutils/CopyableMovableValueType.h @@ -9,6 +9,7 @@ public: CopyableMovableValueType(const CopyableMovableValueType &rhs): CopyableMovableValueType(rhs._value) { ++numCopyConstructorCalled; } + // NOLINTNEXTLINE(cert-oop54-cpp) CopyableMovableValueType &operator=(const CopyableMovableValueType &rhs) { _value = rhs._value; ++numCopyConstructorCalled;