fix some issues found by coverity code scan (#3256)

arangod/Cache/BucketState.cpp

@@ -48,9 +48,9 @@ bool BucketState::isLocked() const {
   return ((_state.load() & static_cast<uint32_t>(Flag::locked)) > 0);
 }
 
-bool BucketState::lock(int64_t maxTries, BucketState::CallbackType cb) {
+bool BucketState::lock(uint64_t maxTries, BucketState::CallbackType cb) {
-  int64_t attempt = 0;
+  uint64_t attempt = 0;
-  while (maxTries < 0 || attempt < maxTries) {
+  while (attempt < maxTries) {
     // expect unlocked, but need to preserve migrating status
     uint32_t current = _state.load(std::memory_order_relaxed);
     uint32_t expected = current & (~static_cast<uint32_t>(Flag::locked));

arangod/Cache/BucketState.h

@@ -97,7 +97,7 @@ struct BucketState {
   /// locked or not. The optional second parameter is a function which will be
   /// called upon successfully locking the state.
   //////////////////////////////////////////////////////////////////////////////
-  bool lock(int64_t maxTries = -1LL, BucketState::CallbackType cb = []() -> void {});
+  bool lock(uint64_t maxTries = UINT64_MAX, BucketState::CallbackType cb = []() -> void {});
 
   //////////////////////////////////////////////////////////////////////////////
   /// @brief Unlocks the state. Requires state to be locked.

arangod/Cache/Cache.cpp

@@ -48,7 +48,7 @@ uint64_t Cache::_findStatsCapacity = 16384;
 
 Cache::ConstructionGuard::ConstructionGuard() {}
 
-Cache::Cache(ConstructionGuard guard, Manager* manager, uint64_t id, Metadata metadata,
+Cache::Cache(ConstructionGuard guard, Manager* manager, uint64_t id, Metadata&& metadata,
              std::shared_ptr<Table> table, bool enableWindowedStats,
              std::function<Table::BucketClearer(Metadata*)> bucketClearer,
              size_t slotsPerBucket)
@@ -60,7 +60,7 @@ Cache::Cache(ConstructionGuard guard, Manager* manager, uint64_t id, Metadata me
       _findMisses(),
       _manager(manager),
       _id(id),
-      _metadata(metadata),
+      _metadata(std::move(metadata)),
       _tableShrdPtr(table),
       _table(table.get()),
       _bucketClearer(bucketClearer(&_metadata)),

arangod/Cache/Cache.h

@@ -72,7 +72,7 @@ class Cache : public std::enable_shared_from_this<Cache> {
   static const uint64_t minLogSize;
 
  public:
-  Cache(ConstructionGuard guard, Manager* manager, uint64_t id, Metadata metadata,
+  Cache(ConstructionGuard guard, Manager* manager, uint64_t id, Metadata&& metadata,
         std::shared_ptr<Table> table, bool enableWindowedStats,
         std::function<Table::BucketClearer(Metadata*)> bucketClearer,
         size_t slotsPerBucket);

arangod/Cache/Manager.cpp

@@ -133,11 +133,11 @@ std::shared_ptr<Cache> Manager::createCache(CacheType type,
   if (allowed) {
     switch (type) {
       case CacheType::Plain:
-        result = PlainCache::create(this, id, metadata, table,
+        result = PlainCache::create(this, id, std::move(metadata), table,
                                     enableWindowedStats);
         break;
       case CacheType::Transactional:
-        result = TransactionalCache::create(this, id, metadata, table,
+        result = TransactionalCache::create(this, id, std::move(metadata), table,
                                             enableWindowedStats);
         break;
       default:

arangod/Cache/PlainBucket.cpp

@@ -35,7 +35,7 @@ PlainBucket::PlainBucket() {
   clear();
 }
 
-bool PlainBucket::lock(int64_t maxTries) { return _state.lock(maxTries); }
+bool PlainBucket::lock(uint64_t maxTries) { return _state.lock(maxTries); }
 
 void PlainBucket::unlock() {
   TRI_ASSERT(_state.isLocked());

arangod/Cache/PlainBucket.h

@@ -67,7 +67,7 @@ struct PlainBucket {
   //////////////////////////////////////////////////////////////////////////////
   /// @brief Attempt to lock bucket (failing after maxTries attempts).
   //////////////////////////////////////////////////////////////////////////////
-  bool lock(int64_t maxTries);
+  bool lock(uint64_t maxTries);
 
   //////////////////////////////////////////////////////////////////////////////
   /// @brief Unlock the bucket. Requires bucket to be locked.

arangod/Cache/PlainCache.cpp

@@ -175,17 +175,17 @@ uint64_t PlainCache::allocationSize(bool enableWindowedStats) {
                               : 0);
 }
 
-std::shared_ptr<Cache> PlainCache::create(Manager* manager, uint64_t id, Metadata metadata,
+std::shared_ptr<Cache> PlainCache::create(Manager* manager, uint64_t id, Metadata&& metadata,
                                           std::shared_ptr<Table> table,
                                           bool enableWindowedStats) {
   return std::make_shared<PlainCache>(Cache::ConstructionGuard(), manager, id,
-                                      metadata, table, enableWindowedStats);
+                                      std::move(metadata), table, enableWindowedStats);
 }
 
 PlainCache::PlainCache(Cache::ConstructionGuard guard, Manager* manager, uint64_t id,
-                       Metadata metadata, std::shared_ptr<Table> table,
+                       Metadata&& metadata, std::shared_ptr<Table> table,
                        bool enableWindowedStats)
-    : Cache(guard, manager, id, metadata, table, enableWindowedStats,
+    : Cache(guard, manager, id, std::move(metadata), table, enableWindowedStats,
             PlainCache::bucketClearer, PlainBucket::slotsData) {}
 
 PlainCache::~PlainCache() {
@@ -233,10 +233,9 @@ void PlainCache::migrateBucket(void* sourcePtr,
   source->lock(Cache::triesGuarantee);
 
   // lock target bucket(s)
-  int64_t tries = Cache::triesGuarantee;
+  targets->applyToAllBuckets([](void* ptr) -> bool {
-  targets->applyToAllBuckets([tries](void* ptr) -> bool {
     auto targetBucket = reinterpret_cast<PlainBucket*>(ptr);
-    return targetBucket->lock(tries);
+    return targetBucket->lock(Cache::triesGuarantee);
   });
 
   for (size_t j = 0; j < PlainBucket::slotsData; j++) {
@@ -287,7 +286,7 @@ void PlainCache::migrateBucket(void* sourcePtr,
 }
 
 std::tuple<Result, PlainBucket*, Table*> PlainCache::getBucket(
-    uint32_t hash, int64_t maxTries, bool singleOperation) {
+    uint32_t hash, uint64_t maxTries, bool singleOperation) {
   Result status;
   PlainBucket* bucket = nullptr;
   Table* source = nullptr;
@@ -315,10 +314,9 @@ std::tuple<Result, PlainBucket*, Table*> PlainCache::getBucket(
 }
 
 Table::BucketClearer PlainCache::bucketClearer(Metadata* metadata) {
-  int64_t tries = Cache::triesGuarantee;
+  return [metadata](void* ptr) -> void {
-  return [metadata, tries](void* ptr) -> void {
     auto bucket = reinterpret_cast<PlainBucket*>(ptr);
-    bucket->lock(tries);
+    bucket->lock(Cache::triesGuarantee);
     for (size_t j = 0; j < PlainBucket::slotsData; j++) {
       if (bucket->_cachedData[j] != nullptr) {
         uint64_t size = bucket->_cachedData[j]->size();

arangod/Cache/PlainCache.h

@@ -54,7 +54,7 @@ namespace cache {
 class PlainCache final : public Cache {
  public:
   PlainCache(Cache::ConstructionGuard guard, Manager* manager, uint64_t id,
-             Metadata metadata, std::shared_ptr<Table> table,
+             Metadata&& metadata, std::shared_ptr<Table> table,
              bool enableWindowedStats);
   ~PlainCache();
 
@@ -105,7 +105,7 @@ class PlainCache final : public Cache {
 
  private:
   static uint64_t allocationSize(bool enableWindowedStats);
-  static std::shared_ptr<Cache> create(Manager* manager, uint64_t id, Metadata metadata,
+  static std::shared_ptr<Cache> create(Manager* manager, uint64_t id, Metadata&& metadata,
                                        std::shared_ptr<Table> table,
                                        bool enableWindowedStats);
 
@@ -116,7 +116,7 @@ class PlainCache final : public Cache {
 
   // helpers
   std::tuple<Result, PlainBucket*, Table*> getBucket(
-      uint32_t hash, int64_t maxTries, bool singleOperation = true);
+      uint32_t hash, uint64_t maxTries, bool singleOperation = true);
   uint32_t getIndex(uint32_t hash, bool useAuxiliary) const;
 
   static Table::BucketClearer bucketClearer(Metadata* metadata);

arangod/Cache/Table.cpp

@@ -34,7 +34,7 @@ using namespace arangodb::cache;
 const uint32_t Table::minLogSize = 8;
 const uint32_t Table::maxLogSize = 32;
 
-bool Table::GenericBucket::lock(int64_t maxTries) {
+bool Table::GenericBucket::lock(uint64_t maxTries) {
   return _state.lock(maxTries);
 }
 
@@ -103,7 +103,7 @@ uint64_t Table::size() const { return _size; }
 uint32_t Table::logSize() const { return _logSize; }
 
 std::pair<void*, Table*> Table::fetchAndLockBucket(
-    uint32_t hash, int64_t maxTries) {
+    uint32_t hash, uint64_t maxTries) {
   GenericBucket* bucket = nullptr;
   Table* source = nullptr;
   bool ok = _lock.readLock(maxTries);
@@ -217,7 +217,7 @@ void Table::enable() {
   _lock.writeUnlock();
 }
 
-bool Table::isEnabled(int64_t maxTries) {
+bool Table::isEnabled(uint64_t maxTries) {
   bool ok = _lock.readLock(maxTries);
   if (ok) {
     ok = !_disabled;

arangod/Cache/Table.h

@@ -45,7 +45,7 @@ class Table : public std::enable_shared_from_this<Table> {
   static const uint32_t minLogSize;
   static const uint32_t maxLogSize;
   static constexpr uint32_t standardLogSizeAdjustment = 6;
-  static constexpr int64_t triesGuarantee = -1;
+  static constexpr uint64_t triesGuarantee = UINT64_MAX;
   static constexpr uint64_t padding = BUCKET_SIZE;
 
   typedef std::function<void(void*)> BucketClearer;
@@ -54,7 +54,7 @@ class Table : public std::enable_shared_from_this<Table> {
   struct GenericBucket {
     BucketState _state;
     uint8_t _filler[BUCKET_SIZE - sizeof(BucketState)];
-    bool lock(int64_t maxTries);
+    bool lock(uint64_t maxTries);
     void unlock();
     bool isMigrated() const;
   };
@@ -119,7 +119,7 @@ class Table : public std::enable_shared_from_this<Table> {
   /// table for the bucket returned as the first member.
   //////////////////////////////////////////////////////////////////////////////
   std::pair<void*, Table*> fetchAndLockBucket(
-      uint32_t hash, int64_t maxTries = -1);
+      uint32_t hash, uint64_t maxTries = UINT64_MAX);
 
   //////////////////////////////////////////////////////////////////////////////
   /// @brief Sets the auxiliary table.
@@ -209,7 +209,7 @@ class Table : public std::enable_shared_from_this<Table> {
 
  private:
   void disable();
-  bool isEnabled(int64_t maxTries = triesGuarantee);
+  bool isEnabled(uint64_t maxTries = triesGuarantee);
   static void defaultClearer(void* ptr);
 };
 

arangod/Cache/TransactionalBucket.cpp

@@ -35,7 +35,7 @@ TransactionalBucket::TransactionalBucket() {
   clear();
 }
 
-bool TransactionalBucket::lock(int64_t maxTries) {
+bool TransactionalBucket::lock(uint64_t maxTries) {
   return _state.lock(maxTries);
 }
 

arangod/Cache/TransactionalBucket.h

@@ -73,7 +73,7 @@ struct TransactionalBucket {
   //////////////////////////////////////////////////////////////////////////////
   /// @brief Attempt to lock bucket (failing after maxTries attempts).
   //////////////////////////////////////////////////////////////////////////////
-  bool lock(int64_t maxTries);
+  bool lock(uint64_t maxTries);
 
   //////////////////////////////////////////////////////////////////////////////
   /// @brief Unlock the bucket. Requires bucket to be locked.

arangod/Cache/TransactionalCache.cpp

@@ -213,19 +213,19 @@ uint64_t TransactionalCache::allocationSize(bool enableWindowedStats) {
 
 std::shared_ptr<Cache> TransactionalCache::create(Manager* manager,
                                                   uint64_t id,
-                                                  Metadata metadata,
+                                                  Metadata&& metadata,
                                                   std::shared_ptr<Table> table,
                                                   bool enableWindowedStats) {
   return std::make_shared<TransactionalCache>(Cache::ConstructionGuard(),
-                                              manager, id, metadata, table,
+                                              manager, id, std::move(metadata), table,
                                               enableWindowedStats);
 }
 
 TransactionalCache::TransactionalCache(Cache::ConstructionGuard guard,
-                                       Manager* manager, uint64_t id, Metadata metadata,
+                                       Manager* manager, uint64_t id, Metadata&& metadata,
                                        std::shared_ptr<Table> table,
                                        bool enableWindowedStats)
-    : Cache(guard, manager, id, metadata, table, enableWindowedStats,
+    : Cache(guard, manager, id, std::move(metadata), table, enableWindowedStats,
             TransactionalCache::bucketClearer, TransactionalBucket::slotsData) {
 }
 
@@ -277,10 +277,9 @@ void TransactionalCache::migrateBucket(void* sourcePtr,
   term = std::max(term, source->_blacklistTerm);
 
   // lock target bucket(s)
-  int64_t tries = Cache::triesGuarantee;
+  targets->applyToAllBuckets([&term](void* ptr) -> bool {
-  targets->applyToAllBuckets([&term, tries](void* ptr) -> bool {
     auto targetBucket = reinterpret_cast<TransactionalBucket*>(ptr);
-    bool locked = targetBucket->lock(tries);
+    bool locked = targetBucket->lock(Cache::triesGuarantee);
     term = std::max(term, targetBucket->_blacklistTerm);
     return locked;
   });
@@ -374,7 +373,7 @@ void TransactionalCache::migrateBucket(void* sourcePtr,
 }
 
 std::tuple<Result, TransactionalBucket*, Table*>
-TransactionalCache::getBucket(uint32_t hash, int64_t maxTries,
+TransactionalCache::getBucket(uint32_t hash, uint64_t maxTries,
                               bool singleOperation) {
   Result status;
   TransactionalBucket* bucket = nullptr;
@@ -405,10 +404,9 @@ TransactionalCache::getBucket(uint32_t hash, int64_t maxTries,
 }
 
 Table::BucketClearer TransactionalCache::bucketClearer(Metadata* metadata) {
-  int64_t tries = Cache::triesGuarantee;
+  return [metadata](void* ptr) -> void {
-  return [metadata, tries](void* ptr) -> void {
     auto bucket = reinterpret_cast<TransactionalBucket*>(ptr);
-    bucket->lock(tries);
+    bucket->lock(Cache::triesGuarantee);
     for (size_t j = 0; j < TransactionalBucket::slotsData; j++) {
       if (bucket->_cachedData[j] != nullptr) {
         uint64_t size = bucket->_cachedData[j]->size();

arangod/Cache/TransactionalCache.h

@@ -62,7 +62,7 @@ namespace cache {
 class TransactionalCache final : public Cache {
  public:
   TransactionalCache(Cache::ConstructionGuard guard, Manager* manager, uint64_t id,
-                     Metadata metadata, std::shared_ptr<Table> table,
+                     Metadata&& metadata, std::shared_ptr<Table> table,
                      bool enableWindowedStats);
   ~TransactionalCache();
 
@@ -121,7 +121,7 @@ class TransactionalCache final : public Cache {
 
  private:
   static uint64_t allocationSize(bool enableWindowedStats);
-  static std::shared_ptr<Cache> create(Manager* manager, uint64_t id, Metadata metadata,
+  static std::shared_ptr<Cache> create(Manager* manager, uint64_t id, Metadata&& metadata,
                                        std::shared_ptr<Table> table,
                                        bool enableWindowedStats);
 
@@ -132,7 +132,7 @@ class TransactionalCache final : public Cache {
 
   // helpers
   std::tuple<Result, TransactionalBucket*, Table*> getBucket(
-      uint32_t hash, int64_t maxTries, bool singleOperation = true);
+      uint32_t hash, uint64_t maxTries, bool singleOperation = true);
   uint32_t getIndex(uint32_t hash, bool useAuxiliary) const;
 
   static Table::BucketClearer bucketClearer(Metadata* metadata);

arangod/Graph/EdgeDocumentToken.h

@@ -37,7 +37,11 @@ namespace graph {
 struct EdgeDocumentToken {
   
   EdgeDocumentToken() noexcept
+#ifdef ARANGODB_ENABLE_MAINTAINER_MODE
+    : _data(0, DocumentIdentifierToken()), _type(TokenType::NONE) {}
+#else
     : _data(0, DocumentIdentifierToken()) {}
+#endif
   
   EdgeDocumentToken(EdgeDocumentToken&& edtkn) noexcept {
 #ifdef ARANGODB_ENABLE_MAINTAINER_MODE

lib/Basics/MutexUnlocker.h

@@ -75,8 +75,8 @@ class MutexUnlocker {
   /// @brief unlocks the mutex if we own it
   bool unlock() {
     if (_isLocked) {
-      _mutex->unlock();
       _isLocked = false;
+      _mutex->unlock();
       return true;
     }
     return false;

lib/Basics/tri-strings.cpp

@@ -659,10 +659,6 @@ char* TRI_SHA256String(char const* source, size_t sourceLen, size_t* dstLen) {
   }
   *dstLen = SHA256_DIGEST_LENGTH;
 
-  if (dst == nullptr) {
-    return nullptr;
-  }
-
   SHA256((unsigned char const*)source, sourceLen, dst);
 
   return (char*)dst;