Merge branch 'unstable' into fix_vector

.clang-tidy

@@ -1,7 +1,7 @@
 # refer to https://clang.llvm.org/extra/clang-tidy/checks/list.html
 Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-deadcode.*, clang-analyzer-nullability.*, clang-analyzer-security.*, clang-analyzer-unix.*, clang-analyzer-valist.*, cppcoreguidelines-init-variables, cppcoreguidelines-macro-usage, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-special-member-functions, cppcoreguidelines-slicing, google-build-explicit-make-pair, google-default-arguments, google-explicit-constructor, modernize-avoid-bind, modernize-loop-convert, modernize-macro-to-enum, modernize-make-shared, modernize-make-unique, modernize-pass-by-value, modernize-redundant-void-arg, modernize-return-braced-init-list, modernize-use-auto, modernize-use-bool-literals, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, modernize-use-nullptr, modernize-use-override, modernize-use-using, performance-faster-string-find, performance-for-range-copy, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-inefficient-vector-operation, performance-move-const-arg, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, performance-unnecessary-value-param
 
-WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-slicing, google-*, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, performance-inefficient-vector-operation
+WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-slicing, google-*, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, modernize-use-bool-literals, performance-unnecessary-value-param, modernize-make-unique, performance-for-range-copy, performance-faster-string-find, modernize-redundant-void-arg, modernize-avoid-bind, modernize-use-auto, modernize-use-using, performance-inefficient-vector-operation
 
 CheckOptions:
   - key:           cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor

src/cluster/cluster.cc

@@ -319,7 +319,9 @@ Status Cluster::ImportSlot(Redis::Connection *conn, int slot, int state) {
       conn->SetImporting();
       myself_->importing_slot_ = slot;
       // Set link error callback
-      conn->close_cb_ = std::bind(&SlotImport::StopForLinkError, svr_->slot_import_, conn->GetFD());
+      conn->close_cb_ = [object_ptr = svr_->slot_import_, capture_fd = conn->GetFD()](int fd) {
+        object_ptr->StopForLinkError(capture_fd);
+      };
       // Stop forbidding writing slot to accept write commands
       if (slot == svr_->slot_migrate_->GetForbiddenSlot()) svr_->slot_migrate_->ReleaseForbiddenSlot();
       LOG(INFO) << "[import] Start importing slot " << slot;

src/commands/redis_cmd.cc

@@ -874,7 +874,7 @@ class CommandDel : public Commander {
   }
 };
 
-Status getBitOffsetFromArgument(std::string arg, uint32_t *offset) {
+Status getBitOffsetFromArgument(const std::string &arg, uint32_t *offset) {
   auto parse_result = ParseInt<uint32_t>(arg, 10);
   if (!parse_result) {
     return parse_result.ToStatus();
@@ -3709,7 +3709,7 @@ class CommandPublish : public Commander {
   }
 };
 
-void SubscribeCommandReply(std::string *output, std::string name, std::string sub_name, int num) {
+void SubscribeCommandReply(std::string *output, const std::string &name, const std::string &sub_name, int num) {
   output->append(Redis::MultiLen(3));
   output->append(Redis::BulkString(name));
   output->append(sub_name.empty() ? Redis::NilString() : Redis::BulkString(sub_name));
@@ -3731,8 +3731,9 @@ class CommandUnSubscribe : public Commander {
  public:
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
     if (args_.size() == 1) {
-      conn->UnSubscribeAll(
-          std::bind(SubscribeCommandReply, output, "unsubscribe", std::placeholders::_1, std::placeholders::_2));
+      conn->UnSubscribeAll([output](const std::string &sub_name, int num) {
+        SubscribeCommandReply(output, "unsubscribe", sub_name, num);
+      });
     } else {
       for (unsigned i = 1; i < args_.size(); i++) {
         conn->UnSubscribeChannel(args_[i]);
@@ -3759,8 +3760,9 @@ class CommandPUnSubscribe : public Commander {
  public:
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
     if (args_.size() == 1) {
-      conn->PUnSubscribeAll(
-          std::bind(SubscribeCommandReply, output, "punsubscribe", std::placeholders::_1, std::placeholders::_2));
+      conn->PUnSubscribeAll([output](const std::string &sub_name, int num) {
+        SubscribeCommandReply(output, "punsubscribe", sub_name, num);
+      });
     } else {
       for (unsigned i = 1; i < args_.size(); i++) {
         conn->PUnSubscribeChannel(args_[i]);
@@ -4715,8 +4717,7 @@ class CommandFetchFile : public Commander {
         // Sleep if the speed of sending file is more than replication speed limit
         auto end = std::chrono::high_resolution_clock::now();
         uint64_t duration = std::chrono::duration_cast<std::chrono::microseconds>(end - start).count();
-        uint64_t shortest =
-            static_cast<uint64_t>(static_cast<double>(file_size) / max_replication_bytes * (1000 * 1000));
+        auto shortest = static_cast<uint64_t>(static_cast<double>(file_size) / max_replication_bytes * (1000 * 1000));
         if (max_replication_bytes > 0 && duration < shortest) {
           LOG(INFO) << "[replication] Need to sleep " << (shortest - duration) / 1000
                     << " ms since of sending files too quickly";

src/common/sha1.cc

@@ -79,10 +79,10 @@ A million repetitions of "a"
 
 void SHA1Transform(uint32_t state[5], const unsigned char buffer[64]) {
   uint32_t a, b, c, d, e;
-  typedef union {
+  union CHAR64LONG16 {
     unsigned char c[64];
     uint32_t l[16];
-  } CHAR64LONG16;
+  };
 #ifdef SHA1HANDSOFF
   CHAR64LONG16 block[1]; /* use array to appear as a pointer */
   memcpy(block, buffer, 64);

src/common/util.cc

@@ -455,7 +455,7 @@ int StringMatchLen(const char *pattern, int patternLen, const char *string, int
           patternLen--;
         }
         match = 0;
-        while (1) {
+        while (true) {
           if (pattern[0] == '\\' && patternLen >= 2) {
             pattern++;
             patternLen--;

src/config/config.cc

@@ -721,7 +721,7 @@ Status Config::Load(const CLIOptions &opts) {
   return finish();
 }
 
-void Config::Get(std::string key, std::vector<std::string> *values) {
+void Config::Get(const std::string &key, std::vector<std::string> *values) {
   values->clear();
   for (const auto &iter : fields_) {
     if (key == "*" || Util::ToLower(key) == iter.first) {

src/config/config.h

@@ -195,7 +195,7 @@ struct Config {
  public:
   Status Rewrite();
   Status Load(const CLIOptions &path);
-  void Get(std::string key, std::vector<std::string> *values);
+  void Get(const std::string &key, std::vector<std::string> *values);
   Status Set(Server *svr, std::string key, const std::string &value);
   void SetMaster(const std::string &host, int port);
   void ClearMaster();

src/server/redis_connection.cc

@@ -195,7 +195,7 @@ void Connection::UnSubscribeChannel(const std::string &channel) {
   }
 }
 
-void Connection::UnSubscribeAll(unsubscribe_callback reply) {
+void Connection::UnSubscribeAll(const unsubscribe_callback &reply) {
   if (subscribe_channels_.empty()) {
     if (reply != nullptr) reply("", subcribe_patterns_.size());
     return;
@@ -232,7 +232,7 @@ void Connection::PUnSubscribeChannel(const std::string &pattern) {
   }
 }
 
-void Connection::PUnSubscribeAll(unsubscribe_callback reply) {
+void Connection::PUnSubscribeAll(const unsubscribe_callback &reply) {
   if (subcribe_patterns_.empty()) {
     if (reply != nullptr) reply("", subscribe_channels_.size());
     return;

src/server/redis_connection.h

@@ -59,11 +59,11 @@ class Connection {
   typedef std::function<void(std::string, int)> unsubscribe_callback;
   void SubscribeChannel(const std::string &channel);
   void UnSubscribeChannel(const std::string &channel);
-  void UnSubscribeAll(unsubscribe_callback reply = nullptr);
+  void UnSubscribeAll(const unsubscribe_callback &reply = nullptr);
   int SubscriptionsCount();
   void PSubscribeChannel(const std::string &pattern);
   void PUnSubscribeChannel(const std::string &pattern);
-  void PUnSubscribeAll(unsubscribe_callback reply = nullptr);
+  void PUnSubscribeAll(const unsubscribe_callback &reply = nullptr);
   int PSubscriptionsCount();
 
   uint64_t GetAge();

src/server/server.cc

@@ -212,7 +212,7 @@ void Server::Join() {
   if (compaction_checker_thread_.joinable()) compaction_checker_thread_.join();
 }
 
-Status Server::AddMaster(std::string host, uint32_t port, bool force_reconnect) {
+Status Server::AddMaster(const std::string &host, uint32_t port, bool force_reconnect) {
   std::lock_guard<std::mutex> guard(slaveof_mu_);
 
   // Don't check host and port if 'force_reconnect' argument is set to true
@@ -230,7 +230,7 @@ Status Server::AddMaster(std::string host, uint32_t port, bool force_reconnect)
   // replication, and uses 'listen-port + 1' as thread listening port.
   uint32_t master_listen_port = port;
   if (GetConfig()->master_use_repl_port) master_listen_port += 1;
-  replication_thread_ = std::unique_ptr<ReplicationThread>(new ReplicationThread(host, master_listen_port, this));
+  replication_thread_ = std::make_unique<ReplicationThread>(host, master_listen_port, this);
   auto s = replication_thread_->Start([this]() { PrepareRestoreDB(); },
                                       [this]() {
                                         this->is_loading_ = false;
@@ -403,7 +403,7 @@ void Server::GetChannelsByPattern(const std::string &pattern, std::vector<std::s
   }
 }
 
-void Server::ListChannelSubscribeNum(std::vector<std::string> channels,
+void Server::ListChannelSubscribeNum(const std::vector<std::string> &channels,
                                      std::vector<ChannelSubscribeNum> *channel_subscribe_nums) {
   std::lock_guard<std::mutex> guard(pubsub_channels_mu_);
   for (const auto &chan : channels) {
@@ -1321,7 +1321,7 @@ std::string Server::GetClientsStr() {
   return clients;
 }
 
-void Server::KillClient(int64_t *killed, std::string addr, uint64_t id, uint64_t type, bool skipme,
+void Server::KillClient(int64_t *killed, const std::string &addr, uint64_t id, uint64_t type, bool skipme,
                         Redis::Connection *conn) {
   *killed = 0;
 

src/server/server.h

@@ -123,7 +123,7 @@ class Server {
   Status LookupAndCreateCommand(const std::string &cmd_name, std::unique_ptr<Redis::Commander> *cmd);
   void AdjustOpenFilesLimit();
 
-  Status AddMaster(std::string host, uint32_t port, bool force_reconnect);
+  Status AddMaster(const std::string &host, uint32_t port, bool force_reconnect);
   Status RemoveMaster();
   Status AddSlave(Redis::Connection *conn, rocksdb::SequenceNumber next_repl_seq);
   void DisconnectSlaves();
@@ -138,7 +138,7 @@ class Server {
   void SubscribeChannel(const std::string &channel, Redis::Connection *conn);
   void UnSubscribeChannel(const std::string &channel, Redis::Connection *conn);
   void GetChannelsByPattern(const std::string &pattern, std::vector<std::string> *channels);
-  void ListChannelSubscribeNum(std::vector<std::string> channels,
+  void ListChannelSubscribeNum(const std::vector<std::string> &channels,
                                std::vector<ChannelSubscribeNum> *channel_subscribe_nums);
   void PSubscribeChannel(const std::string &pattern, Redis::Connection *conn);
   void PUnSubscribeChannel(const std::string &pattern, Redis::Connection *conn);
@@ -185,7 +185,8 @@ class Server {
   int DecrBlockedClientNum();
   std::string GetClientsStr();
   std::atomic<uint64_t> *GetClientID();
-  void KillClient(int64_t *killed, std::string addr, uint64_t id, uint64_t type, bool skipme, Redis::Connection *conn);
+  void KillClient(int64_t *killed, const std::string &addr, uint64_t id, uint64_t type, bool skipme,
+                  Redis::Connection *conn);
 
   lua_State *Lua() { return lua_; }
   Status ScriptExists(const std::string &sha);

src/server/worker.cc

@@ -423,7 +423,7 @@ std::string Worker::GetClientsStr() {
   return output;
 }
 
-void Worker::KillClient(Redis::Connection *self, uint64_t id, std::string addr, uint64_t type, bool skipme,
+void Worker::KillClient(Redis::Connection *self, uint64_t id, const std::string &addr, uint64_t type, bool skipme,
                         int64_t *killed) {
   std::lock_guard<std::mutex> guard(conns_mu_);
   for (const auto &iter : conns_) {

src/server/worker.h

@@ -60,7 +60,8 @@ class Worker {
   void FeedMonitorConns(Redis::Connection *conn, const std::vector<std::string> &tokens);
 
   std::string GetClientsStr();
-  void KillClient(Redis::Connection *self, uint64_t id, std::string addr, uint64_t type, bool skipme, int64_t *killed);
+  void KillClient(Redis::Connection *self, uint64_t id, const std::string &addr, uint64_t type, bool skipme,
+                  int64_t *killed);
   void KickoutIdleClients(int timeout);
 
   Status ListenUnixSocket(const std::string &path, int perm, int backlog);

src/storage/batch_extractor.cc

@@ -70,7 +70,7 @@ rocksdb::Status WriteBatchExtractor::PutCF(uint32_t column_family_id, const Slic
         if (!parse_result) {
           return rocksdb::Status::InvalidArgument(parse_result.Msg());
         }
-        RedisCommand cmd = static_cast<RedisCommand>(*parse_result);
+        auto cmd = static_cast<RedisCommand>(*parse_result);
         if (cmd == kRedisCmdExpire) {
           command_args = {"EXPIREAT", user_key, std::to_string(metadata.expire)};
           resp_commands_[ns].emplace_back(Redis::Command2RESP(command_args));
@@ -103,7 +103,7 @@ rocksdb::Status WriteBatchExtractor::PutCF(uint32_t column_family_id, const Slic
         if (!parse_result) {
           return rocksdb::Status::InvalidArgument(parse_result.Msg());
         }
-        RedisCommand cmd = static_cast<RedisCommand>(*parse_result);
+        auto cmd = static_cast<RedisCommand>(*parse_result);
         switch (cmd) {
           case kRedisCmdLSet:
             if (args->size() < 2) {
@@ -149,7 +149,7 @@ rocksdb::Status WriteBatchExtractor::PutCF(uint32_t column_family_id, const Slic
         if (!parse_result) {
           return rocksdb::Status::InvalidArgument(parse_result.Msg());
         }
-        RedisCommand cmd = static_cast<RedisCommand>(*parse_result);
+        auto cmd = static_cast<RedisCommand>(*parse_result);
         switch (cmd) {
           case kRedisCmdSetBit: {
             if (args->size() < 2) {
@@ -240,7 +240,7 @@ rocksdb::Status WriteBatchExtractor::DeleteCF(uint32_t column_family_id, const S
         if (!parse_result) {
           return rocksdb::Status::InvalidArgument(parse_result.Msg());
         }
-        RedisCommand cmd = static_cast<RedisCommand>(*parse_result);
+        auto cmd = static_cast<RedisCommand>(*parse_result);
         switch (cmd) {
           case kRedisCmdLTrim:
             if (first_seen_) {

src/storage/redis_db.cc

@@ -163,7 +163,7 @@ rocksdb::Status Database::TTL(const Slice &user_key, int *ttl) {
 
 void Database::GetKeyNumStats(const std::string &prefix, KeyNumStats *stats) { Keys(prefix, nullptr, stats); }
 
-void Database::Keys(std::string prefix, std::vector<std::string> *keys, KeyNumStats *stats) {
+void Database::Keys(const std::string &prefix, std::vector<std::string> *keys, KeyNumStats *stats) {
   uint16_t slot_id = 0;
   std::string ns_prefix, ns, user_key, value;
   if (namespace_ != kDefaultNamespace || keys != nullptr) {

src/storage/redis_db.h

@@ -44,7 +44,7 @@ class Database {
   rocksdb::Status FlushDB();
   rocksdb::Status FlushAll();
   void GetKeyNumStats(const std::string &prefix, KeyNumStats *stats);
-  void Keys(std::string prefix, std::vector<std::string> *keys = nullptr, KeyNumStats *stats = nullptr);
+  void Keys(const std::string &prefix, std::vector<std::string> *keys = nullptr, KeyNumStats *stats = nullptr);
   rocksdb::Status Scan(const std::string &cursor, uint64_t limit, const std::string &prefix,
                        std::vector<std::string> *keys, std::string *end_cursor = nullptr);
   rocksdb::Status RandomKey(const std::string &cursor, std::string *key);

src/storage/storage.cc

@@ -643,7 +643,7 @@ Status Storage::WriteToPropagateCF(const std::string &key, const std::string &va
   return Status::OK();
 }
 
-bool Storage::ShiftReplId(void) {
+bool Storage::ShiftReplId() {
   const char *charset = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
   const int charset_len = strlen(charset);
 
@@ -695,7 +695,7 @@ std::string Storage::GetReplIdFromWalBySeq(rocksdb::SequenceNumber seq) {
         }
       }
     };
-    std::string GetReplId(void) { return replid_in_wal_; }
+    std::string GetReplId() { return replid_in_wal_; }
 
    private:
     std::string replid_in_wal_;
@@ -708,7 +708,7 @@ std::string Storage::GetReplIdFromWalBySeq(rocksdb::SequenceNumber seq) {
   return write_batch_handler.GetReplId();
 }
 
-std::string Storage::GetReplIdFromDbEngine(void) {
+std::string Storage::GetReplIdFromDbEngine() {
   std::string replid_in_db;
   auto cf = GetCFHandle(kPropagateColumnFamilyName);
   auto s = db_->Get(rocksdb::ReadOptions(), cf, kReplicationIdKey, &replid_in_db);
@@ -766,7 +766,7 @@ Status Storage::ReplDataManager::GetFullReplDataInfo(Storage *storage, std::stri
   // Get checkpoint file list
   std::vector<std::string> result;
   storage->env_->GetChildren(data_files_dir, &result);
-  for (auto f : result) {
+  for (const auto &f : result) {
     if (f == "." || f == "..") continue;
     files->append(f);
     files->push_back(',');
@@ -775,12 +775,12 @@ Status Storage::ReplDataManager::GetFullReplDataInfo(Storage *storage, std::stri
   return Status::OK();
 }
 
-bool Storage::ExistCheckpoint(void) {
+bool Storage::ExistCheckpoint() {
   std::lock_guard<std::mutex> lg(checkpoint_mu_);
   return env_->FileExists(config_->checkpoint_dir).ok();
 }
 
-bool Storage::ExistSyncCheckpoint(void) { return env_->FileExists(config_->sync_checkpoint_dir).ok(); }
+bool Storage::ExistSyncCheckpoint() { return env_->FileExists(config_->sync_checkpoint_dir).ok(); }
 
 Status Storage::ReplDataManager::CleanInvalidFiles(Storage *storage, const std::string &dir,
                                                    std::vector<std::string> valid_files) {
@@ -790,7 +790,7 @@ Status Storage::ReplDataManager::CleanInvalidFiles(Storage *storage, const std::
 
   std::vector<std::string> tmp_files, files;
   storage->env_->GetChildren(dir, &tmp_files);
-  for (auto file : tmp_files) {
+  for (const auto &file : tmp_files) {
     if (file == "." || file == "..") continue;
     files.push_back(file);
   }
@@ -946,8 +946,7 @@ bool Storage::ReplDataManager::FileExists(Storage *storage, const std::string &d
   uint64_t size;
   s = storage->env_->GetFileSize(file_path, &size);
   if (!s.ok()) return false;
-  std::unique_ptr<rocksdb::SequentialFileWrapper> src_reader;
-  src_reader.reset(new rocksdb::SequentialFileWrapper(src_file.get()));
+  auto src_reader = std::make_unique<rocksdb::SequentialFileWrapper>(src_file.get());
 
   char buffer[4096];
   Slice slice;