Enable some more clang-tidy checks and fix warnings

.clang-tidy

@@ -1,5 +1,28 @@
 ---
-Checks:          'clang-diagnostic-*,clang-analyzer-*,misc-use-after-move,misc-unused-using-decls,misc-unused-alias-decls'
+# 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?
+Checks: |
+  clang-diagnostic-*,
+  clang-analyzer-*,
+  bugprone-*,
+  cert-*,
+  cppcoreguidelines-*,
+  misc-*,
+  boost-use-to-string,
+  -cert-env33-c,
+  -cert-err58-cpp,
+  -cppcoreguidelines-owning-memory,
+  -cppcoreguidelines-no-malloc,
+  -cppcoreguidelines-pro-type-const-cast,
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+  -cppcoreguidelines-pro-type-reinterpret-cast,
+  -cppcoreguidelines-special-member-functions,
+  -cppcoreguidelines-pro-type-cstyle-cast,
+  -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
+  -cppcoreguidelines-pro-type-vararg,
+  -misc-macro-parentheses,
+  -misc-unused-raii
 WarningsAsErrors: ''
 HeaderFilterRegex: '/src/|/test/'
 CheckOptions:

src/blobstore/implementations/onblocks/datanodestore/DataNodeView.h

@@ -69,7 +69,7 @@ public:
     ASSERT(data.size() <= layout.datasizeBytes(), "Data is too large for node");
     cpputils::Data serialized = _serialize(layout, formatVersion, depth, size, std::move(data));
     ASSERT(serialized.size() == layout.blocksizeBytes(), "Wrong block size");
-    auto block = blockStore->create(std::move(serialized));
+    auto block = blockStore->create(serialized);
     return DataNodeView(std::move(block));
   }
 

src/blobstore/implementations/onblocks/parallelaccessdatatreestore/ParallelAccessDataTreeStoreAdapter.h

@@ -14,7 +14,7 @@ namespace parallelaccessdatatreestore {
 class ParallelAccessDataTreeStoreAdapter final: public parallelaccessstore::ParallelAccessBaseStore<datatreestore::DataTree, blockstore::BlockId> {
 public:
   ParallelAccessDataTreeStoreAdapter(datatreestore::DataTreeStore *baseDataTreeStore)
-    :_baseDataTreeStore(std::move(baseDataTreeStore)) {
+    :_baseDataTreeStore(baseDataTreeStore) {
   }
 
   boost::optional<cpputils::unique_ref<datatreestore::DataTree>> loadFromBaseStore(const blockstore::BlockId &blockId) override {

src/blockstore/implementations/caching/cache/CacheEntry.h

@@ -16,7 +16,7 @@ public:
   explicit CacheEntry(Value value): _lastAccess(currentTime()), _value(std::move(value)) {
   }
 
-  CacheEntry(CacheEntry &&) = default;
+  CacheEntry(CacheEntry &&) noexcept = default;
 
   double ageSeconds() const {
     return ((double)(currentTime() - _lastAccess).total_nanoseconds()) / ((double)1000000000);

src/blockstore/implementations/integrity/ClientIdAndBlockId.h

@@ -8,8 +8,8 @@
 namespace blockstore {
     namespace integrity {
 
-        struct ClientIdAndBlockId {
+        struct ClientIdAndBlockId final {
-            uint32_t clientId;
+            uint32_t clientId = 0;
             BlockId blockId;
         };
 

src/blockstore/implementations/integrity/IntegrityBlockStore2.cpp

@@ -215,8 +215,8 @@ void IntegrityBlockStore2::migrateBlockFromBlockstoreWithoutVersionNumbers(block
     return;
   }
   cpputils::Data data = std::move(*data_);
-  cpputils::Data dataWithHeader = _prependHeaderToData(blockId, knownBlockVersions->myClientId(), version, std::move(data));
+  cpputils::Data dataWithHeader = _prependHeaderToData(blockId, knownBlockVersions->myClientId(), version, data);
-  baseBlockStore->store(blockId, std::move(dataWithHeader));
+  baseBlockStore->store(blockId, dataWithHeader);
 }
 #endif
 

src/blockstore/implementations/integrity/KnownBlockVersions.cpp

@@ -27,7 +27,7 @@ KnownBlockVersions::KnownBlockVersions(const bf::path &stateFilePath, uint32_t m
     _loadStateFile();
 }
 
-KnownBlockVersions::KnownBlockVersions(KnownBlockVersions &&rhs)
+KnownBlockVersions::KnownBlockVersions(KnownBlockVersions &&rhs) // NOLINT (intentionally not noexcept)
         : _knownVersions(), _lastUpdateClientId(), _stateFilePath(), _myClientId(0), _mutex(), _valid(true) {
     unique_lock<mutex> rhsLock(rhs._mutex);
     unique_lock<mutex> lock(_mutex);

src/blockstore/implementations/integrity/KnownBlockVersions.h

@@ -18,7 +18,7 @@ namespace blockstore {
         class KnownBlockVersions final {
         public:
             KnownBlockVersions(const boost::filesystem::path &stateFilePath, uint32_t myClientId);
-            KnownBlockVersions(KnownBlockVersions &&rhs);
+            KnownBlockVersions(KnownBlockVersions &&rhs); // NOLINT (intentionally not noexcept)
             ~KnownBlockVersions();
 
             __attribute__((warn_unused_result))

src/blockstore/implementations/ondisk/OnDiskBlockStore2.cpp

@@ -76,7 +76,7 @@ optional<Data> OnDiskBlockStore2::load(const BlockId &blockId) const {
   if (fileContent == none) {
     return boost::none;
   }
-  return _checkAndRemoveHeader(std::move(*fileContent));
+  return _checkAndRemoveHeader(*fileContent);
 }
 
 void OnDiskBlockStore2::store(const BlockId &blockId, const Data &data) {
@@ -99,7 +99,7 @@ uint64_t OnDiskBlockStore2::numBlocks() const {
 }
 
 uint64_t OnDiskBlockStore2::estimateNumFreeBytes() const {
-  struct statvfs stat;
+  struct statvfs stat{};
   int result = ::statvfs(_rootDir.c_str(), &stat);
   if (0 != result) {
     throw std::runtime_error("Error calling statvfs()");

src/blockstore/implementations/parallelaccess/ParallelAccessBlockStoreAdapter.h

@@ -12,7 +12,7 @@ namespace parallelaccess {
 class ParallelAccessBlockStoreAdapter final: public parallelaccessstore::ParallelAccessBaseStore<Block, BlockId> {
 public:
   explicit ParallelAccessBlockStoreAdapter(BlockStore *baseBlockStore)
-    :_baseBlockStore(std::move(baseBlockStore)) {
+    :_baseBlockStore(baseBlockStore) {
   }
 
   boost::optional<cpputils::unique_ref<Block>> loadFromBaseStore(const BlockId &blockId) override {

src/cpp-utils/crypto/hash/Hash.cpp

@@ -17,8 +17,8 @@ Hash hash(const Data& data, Salt salt) {
   hasher.Final((CryptoPP::byte*)digest.data());
 
   return Hash{
-      .digest = std::move(digest),
+      .digest = digest,
-      .salt = std::move(salt)
+      .salt = salt
   };
 }
 

src/cpp-utils/data/Data.h

@@ -17,8 +17,8 @@ public:
   explicit Data(size_t size);
   ~Data();
 
-  Data(Data &&rhs); // move constructor
+  Data(Data &&rhs) noexcept;
-  Data &operator=(Data &&rhs); // move assignment
+  Data &operator=(Data &&rhs) noexcept;
 
   Data copy() const;
 
@@ -81,14 +81,14 @@ inline Data::Data(size_t size)
   }
 }
 
-inline Data::Data(Data &&rhs)
+inline Data::Data(Data &&rhs) noexcept
         : _size(rhs._size), _data(rhs._data) {
   // Make rhs invalid, so the memory doesn't get freed in its destructor.
   rhs._data = nullptr;
   rhs._size = 0;
 }
 
-inline Data &Data::operator=(Data &&rhs) {
+inline Data &Data::operator=(Data &&rhs) noexcept {
   std::free(_data);
   _data = rhs._data;
   _size = rhs._size;

src/cpp-utils/data/FixedSizeData.h

@@ -34,7 +34,7 @@ public:
   template<size_t size> FixedSizeData<SIZE-size> drop() const;
 
 private:
-  FixedSizeData() {}
+  FixedSizeData(): _data() {}
   template<size_t _SIZE> friend class FixedSizeData;
 
   unsigned char _data[BINARY_LENGTH];

src/cpp-utils/io/DontEchoStdinToStdoutRAII.h

@@ -17,7 +17,7 @@ namespace cpputils {
  */
 class DontEchoStdinToStdoutRAII final {
 public:
-    DontEchoStdinToStdoutRAII() {
+    DontEchoStdinToStdoutRAII(): _old_state() {
         tcgetattr(STDIN_FILENO, &_old_state);
         termios new_state = _old_state;
         new_state.c_lflag &= ~ECHO;

src/cpp-utils/lock/MutexPoolLock.h

@@ -17,7 +17,7 @@ namespace cpputils {
             _pool->lock(_lockName, lockToFreeWhileWaiting);
         }
         
-        MutexPoolLock(MutexPoolLock &&rhs): _pool(rhs._pool), _lockName(rhs._lockName) {
+        MutexPoolLock(MutexPoolLock &&rhs) noexcept: _pool(rhs._pool), _lockName(std::move(rhs._lockName)) {
             rhs._pool = nullptr;
         }
 

src/cpp-utils/pointer/unique_ref.h

@@ -161,7 +161,7 @@ inline bool operator!=(const unique_ref<T, D> &lhs, const unique_ref<T, D> &rhs)
 
 }
 
-namespace std {
+namespace std { // NOLINT (intentional change of namespace std)
     template<class T, class D>
     inline void swap(cpputils::unique_ref<T, D>& lhs, cpputils::unique_ref<T, D>& rhs) noexcept {
         lhs.swap(rhs);

src/cpp-utils/system/time.h

@@ -11,7 +11,7 @@ namespace cpputils {
     namespace time {
 
         inline timespec now() {
-            struct timespec now;
+            struct timespec now{};
             clock_gettime(CLOCK_REALTIME, &now);
             return now;
         }

src/cpp-utils/thread/ThreadSystem.cpp

@@ -62,7 +62,7 @@ namespace cpputils {
 
     boost::thread ThreadSystem::_startThread(function<bool()> loopIteration) {
         return boost::thread([loopIteration = std::move(loopIteration)] {
-            ThreadSystem::_runThread(std::move(loopIteration));
+            ThreadSystem::_runThread(loopIteration);
         });
     }
 

src/cryfs/config/CryConfig.cpp

@@ -136,7 +136,7 @@ const CryConfig::FilesystemID &CryConfig::FilesystemId() const {
 }
 
 void CryConfig::SetFilesystemId(FilesystemID value) {
-  _filesystemId = std::move(value);
+  _filesystemId = value;
 }
 
 optional<uint32_t> CryConfig::ExclusiveClientId() const {

src/cryfs/config/crypto/CryConfigEncryptorFactory.cpp

@@ -34,6 +34,6 @@ namespace cryfs {
         //     This would need a change in the scrypt interface though, because right now we can't continue past key computations.
         //TODO I might be able to know the actual key size here (at runtime) and switch the SCrypt deriveKey() interface to getting a dynamic size.
         auto key = kdf->deriveKey<CryConfigEncryptor::MaxTotalKeySize>(password);
-        return make_unique_ref<CryConfigEncryptor>(std::move(key), kdf->kdfParameters().copy());
+        return make_unique_ref<CryConfigEncryptor>(key, kdf->kdfParameters().copy());
     }
 }

src/cryfs/filesystem/CryNode.cpp

@@ -170,7 +170,7 @@ void CryNode::stat(struct ::stat *result) const {
     result->st_size = fsblobstore::DirBlob::DIR_LSTAT_SIZE;
     //TODO If possible without performance loss, then for a directory, st_nlink should return number of dir entries (including "." and "..")
     result->st_nlink = 1;
-    struct timespec now;
+    struct timespec now{};
     clock_gettime(CLOCK_REALTIME, &now);
     result->st_atim = now;
     result->st_mtim = now;

src/cryfs/filesystem/fsblobstore/utils/DirEntry.cpp

@@ -57,7 +57,7 @@ namespace cryfs {
         }
 
         timespec DirEntry::_deserializeTimeValue(const char **pos) {
-            timespec value;
+            timespec value{};
             value.tv_sec = *(uint64_t*)(*pos);
             *pos += sizeof(uint64_t);
             value.tv_nsec = *(uint32_t*)(*pos);

src/cryfs/filesystem/parallelaccessfsblobstore/ParallelAccessFsBlobStoreAdapter.h

@@ -12,7 +12,7 @@ namespace parallelaccessfsblobstore {
 class ParallelAccessFsBlobStoreAdapter final: public parallelaccessstore::ParallelAccessBaseStore<cachingfsblobstore::FsBlobRef, blockstore::BlockId> {
 public:
   explicit ParallelAccessFsBlobStoreAdapter(cachingfsblobstore::CachingFsBlobStore *baseBlobStore)
-    :_baseBlobStore(std::move(baseBlobStore)) {
+    :_baseBlobStore(baseBlobStore) {
   }
 
   boost::optional<cpputils::unique_ref<cachingfsblobstore::FsBlobRef>> loadFromBaseStore(const blockstore::BlockId &blockId) override {

src/cryfs/localstate/BasedirMetadata.cpp

@@ -43,7 +43,7 @@ string jsonPathForBasedir(const bf::path &basedir) {
 }
 
 BasedirMetadata::BasedirMetadata(ptree data)
-  :_data(std::move(data)) {}
+  :_data(data) {}
 
 BasedirMetadata BasedirMetadata::load() {
   return BasedirMetadata(_load(LocalStateDir::forBasedirMetadata()));

src/cryfs/localstate/LocalStateMetadata.cpp

@@ -24,7 +24,7 @@ namespace bf = boost::filesystem;
 namespace cryfs {
 
 LocalStateMetadata::LocalStateMetadata(uint32_t myClientId, Hash encryptionKeyHash)
-: _myClientId(myClientId), _encryptionKeyHash(std::move(encryptionKeyHash)) {}
+: _myClientId(myClientId), _encryptionKeyHash(encryptionKeyHash) {}
 
 LocalStateMetadata LocalStateMetadata::loadOrGenerate(const bf::path &statePath, const Data& encryptionKey) {
   auto metadataFile = statePath / "metadata";
@@ -113,8 +113,8 @@ LocalStateMetadata LocalStateMetadata::_deserialize(istream& stream) {
   string encryptionKeyDigest = pt.get<string>("encryptionKey.hash");
 
   return LocalStateMetadata(myClientId, Hash{
-      .digest = cpputils::hash::Digest::FromString(std::move(encryptionKeyDigest)),
+      .digest = cpputils::hash::Digest::FromString(encryptionKeyDigest),
-      .salt = cpputils::hash::Salt::FromString(std::move(encryptionKeySalt))
+      .salt = cpputils::hash::Salt::FromString(encryptionKeySalt)
   });
 }
 

src/fspp/fuse/Fuse.cpp

@@ -778,7 +778,7 @@ int Fuse::readdir(const bf::path &path, void *buf, fuse_fill_dir_t filler, off_t
   UNUSED(offset);
   try {
     auto entries = _fs->readDir(path);
-    struct stat stbuf;
+    struct stat stbuf{};
     for (const auto &entry : *entries) {
       //We could pass more file metadata to filler() in its third parameter,
       //but it doesn't help performance since fuse ignores everything in stbuf

src/gitversion/parser.h

@@ -8,14 +8,14 @@
 
 namespace gitversion {
     struct VersionInfo {
-        bool isDevVersion;
+        bool isDevVersion = false;
-        bool isStableVersion;
+        bool isStableVersion = false;
         std::string versionTag;
         std::string gitCommitId;
         std::string majorVersion;
         std::string minorVersion;
         std::string hotfixVersion;
-        unsigned int commitsSinceTag;
+        unsigned int commitsSinceTag = 0;
     };
 
     class Parser final {

src/parallelaccessstore/ParallelAccessStore.h

@@ -63,7 +63,7 @@ private:
   class OpenResource final {
   public:
 	OpenResource(cpputils::unique_ref<Resource> resource): _resource(std::move(resource)), _refCount(0) {}
-    OpenResource(OpenResource &&rhs) = default;
+    OpenResource(OpenResource &&rhs) noexcept = default;
 
 	Resource *getReference() {
 	  ++_refCount;