From f48b354ce536b6a98ba3a44ec2e7b726279e6bd5 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Sun, 13 Nov 2022 00:40:51 +0800 Subject: [PATCH 1/3] Fix performance-unnecessary-value-param warning reported by clang-tidy --- .clang-tidy | 3 ++- src/commands/redis_cmd.cc | 4 ++-- src/config/config.cc | 2 +- src/config/config.h | 2 +- src/server/redis_connection.cc | 2 +- src/server/redis_connection.h | 2 +- src/server/server.cc | 6 +++--- src/server/server.h | 7 ++++--- src/server/worker.cc | 2 +- src/server/worker.h | 3 ++- src/storage/redis_db.cc | 2 +- src/storage/redis_db.h | 2 +- src/types/redis_list.cc | 2 +- src/types/redis_list.h | 3 ++- src/types/redis_sortedint.cc | 6 +++--- src/types/redis_sortedint.h | 6 +++--- src/types/redis_zset.cc | 2 +- src/types/redis_zset.h | 3 ++- 18 files changed, 32 insertions(+), 27 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 4f70482cacc..dcc013c7db6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,7 +1,8 @@ # 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 +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-unnecessary-value-param CheckOptions: - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor diff --git a/src/commands/redis_cmd.cc b/src/commands/redis_cmd.cc index 941ac4cc94f..3b477174e9e 100644 --- a/src/commands/redis_cmd.cc +++ b/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(arg, 10); if (!parse_result) { return parse_result.ToStatus(); @@ -3707,7 +3707,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)); diff --git a/src/config/config.cc b/src/config/config.cc index 354448f6603..62e8cce00a5 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -721,7 +721,7 @@ Status Config::Load(const CLIOptions &opts) { return finish(); } -void Config::Get(std::string key, std::vector *values) { +void Config::Get(const std::string &key, std::vector *values) { values->clear(); for (const auto &iter : fields_) { if (key == "*" || Util::ToLower(key) == iter.first) { diff --git a/src/config/config.h b/src/config/config.h index 12d2a9d1f78..e57145260b0 100644 --- a/src/config/config.h +++ b/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 *values); + void Get(const std::string &key, std::vector *values); Status Set(Server *svr, std::string key, const std::string &value); void SetMaster(const std::string &host, int port); void ClearMaster(); diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index 38f952439f6..5cd444a3347 100644 --- a/src/server/redis_connection.cc +++ b/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; diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index f57911283db..e6cb7e08d1b 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -59,7 +59,7 @@ class Connection { typedef std::function 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); diff --git a/src/server/server.cc b/src/server/server.cc index f879165dc79..13614f5e8a9 100644 --- a/src/server/server.cc +++ b/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 guard(slaveof_mu_); // Don't check host and port if 'force_reconnect' argument is set to true @@ -403,7 +403,7 @@ void Server::GetChannelsByPattern(const std::string &pattern, std::vector channels, +void Server::ListChannelSubscribeNum(const std::vector &channels, std::vector *channel_subscribe_nums) { std::lock_guard 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; diff --git a/src/server/server.h b/src/server/server.h index 2b2e2219127..7c75adeb976 100644 --- a/src/server/server.h +++ b/src/server/server.h @@ -123,7 +123,7 @@ class Server { Status LookupAndCreateCommand(const std::string &cmd_name, std::unique_ptr *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 *channels); - void ListChannelSubscribeNum(std::vector channels, + void ListChannelSubscribeNum(const std::vector &channels, std::vector *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 *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); diff --git a/src/server/worker.cc b/src/server/worker.cc index 35e81c53311..f19ec2ae778 100644 --- a/src/server/worker.cc +++ b/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 guard(conns_mu_); for (const auto &iter : conns_) { diff --git a/src/server/worker.h b/src/server/worker.h index 6ae484ed7d6..f2f53e2254d 100644 --- a/src/server/worker.h +++ b/src/server/worker.h @@ -60,7 +60,8 @@ class Worker { void FeedMonitorConns(Redis::Connection *conn, const std::vector &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); diff --git a/src/storage/redis_db.cc b/src/storage/redis_db.cc index d43b7925959..75cbf376bbe 100644 --- a/src/storage/redis_db.cc +++ b/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 *keys, KeyNumStats *stats) { +void Database::Keys(const std::string &prefix, std::vector *keys, KeyNumStats *stats) { uint16_t slot_id = 0; std::string ns_prefix, ns, user_key, value; if (namespace_ != kDefaultNamespace || keys != nullptr) { diff --git a/src/storage/redis_db.h b/src/storage/redis_db.h index 1a44449b304..28b2be2164f 100644 --- a/src/storage/redis_db.h +++ b/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 *keys = nullptr, KeyNumStats *stats = nullptr); + void Keys(const std::string &prefix, std::vector *keys = nullptr, KeyNumStats *stats = nullptr); rocksdb::Status Scan(const std::string &cursor, uint64_t limit, const std::string &prefix, std::vector *keys, std::string *end_cursor = nullptr); rocksdb::Status RandomKey(const std::string &cursor, std::string *key); diff --git a/src/types/redis_list.cc b/src/types/redis_list.cc index b0c51e63391..6ed673d2da6 100644 --- a/src/types/redis_list.cc +++ b/src/types/redis_list.cc @@ -51,7 +51,7 @@ rocksdb::Status List::PushX(const Slice &user_key, const std::vector &ele return push(user_key, elems, false, left, ret); } -rocksdb::Status List::push(const Slice &user_key, std::vector elems, bool create_if_missing, bool left, +rocksdb::Status List::push(const Slice &user_key, const std::vector &elems, bool create_if_missing, bool left, int *ret) { *ret = 0; std::string ns_key; diff --git a/src/types/redis_list.h b/src/types/redis_list.h index f3a3077a769..050fa75fe4a 100644 --- a/src/types/redis_list.h +++ b/src/types/redis_list.h @@ -49,7 +49,8 @@ class List : public Database { private: rocksdb::Status GetMetadata(const Slice &ns_key, ListMetadata *metadata); - rocksdb::Status push(const Slice &user_key, std::vector elems, bool create_if_missing, bool left, int *ret); + rocksdb::Status push(const Slice &user_key, const std::vector &elems, bool create_if_missing, bool left, + int *ret); rocksdb::Status lmoveOnSingleList(const Slice &src, bool src_left, bool dst_left, std::string *elem); rocksdb::Status lmoveOnTwoLists(const Slice &src, const Slice &dst, bool src_left, bool dst_left, std::string *elem); }; diff --git a/src/types/redis_sortedint.cc b/src/types/redis_sortedint.cc index e9dac4490e9..da92bb7e313 100644 --- a/src/types/redis_sortedint.cc +++ b/src/types/redis_sortedint.cc @@ -33,7 +33,7 @@ rocksdb::Status Sortedint::GetMetadata(const Slice &ns_key, SortedintMetadata *m return Database::GetMetadata(kRedisSortedint, ns_key, metadata); } -rocksdb::Status Sortedint::Add(const Slice &user_key, std::vector ids, int *ret) { +rocksdb::Status Sortedint::Add(const Slice &user_key, const std::vector &ids, int *ret) { *ret = 0; std::string ns_key; @@ -67,7 +67,7 @@ rocksdb::Status Sortedint::Add(const Slice &user_key, std::vector ids, return storage_->Write(storage_->DefaultWriteOptions(), &batch); } -rocksdb::Status Sortedint::Remove(const Slice &user_key, std::vector ids, int *ret) { +rocksdb::Status Sortedint::Remove(const Slice &user_key, const std::vector &ids, int *ret) { *ret = 0; std::string ns_key; @@ -210,7 +210,7 @@ rocksdb::Status Sortedint::RangeByValue(const Slice &user_key, SortedintRangeSpe return rocksdb::Status::OK(); } -rocksdb::Status Sortedint::MExist(const Slice &user_key, std::vector ids, std::vector *exists) { +rocksdb::Status Sortedint::MExist(const Slice &user_key, const std::vector &ids, std::vector *exists) { std::string ns_key; AppendNamespacePrefix(user_key, &ns_key); diff --git a/src/types/redis_sortedint.h b/src/types/redis_sortedint.h index 0c0f2174c1b..e37d85226a9 100644 --- a/src/types/redis_sortedint.h +++ b/src/types/redis_sortedint.h @@ -48,9 +48,9 @@ class Sortedint : public Database { public: explicit Sortedint(Engine::Storage *storage, const std::string &ns) : Database(storage, ns) {} rocksdb::Status Card(const Slice &user_key, int *ret); - rocksdb::Status MExist(const Slice &user_key, std::vector ids, std::vector *exists); - rocksdb::Status Add(const Slice &user_key, std::vector ids, int *ret); - rocksdb::Status Remove(const Slice &user_key, std::vector ids, int *ret); + rocksdb::Status MExist(const Slice &user_key, const std::vector &ids, std::vector *exists); + rocksdb::Status Add(const Slice &user_key, const std::vector &ids, int *ret); + rocksdb::Status Remove(const Slice &user_key, const std::vector &ids, int *ret); rocksdb::Status Range(const Slice &user_key, uint64_t cursor_id, uint64_t page, uint64_t limit, bool reversed, std::vector *ids); rocksdb::Status RangeByValue(const Slice &user_key, SortedintRangeSpec spec, std::vector *ids, int *size); diff --git a/src/types/redis_zset.cc b/src/types/redis_zset.cc index 558f25ab27b..e4c8eb44d71 100644 --- a/src/types/redis_zset.cc +++ b/src/types/redis_zset.cc @@ -415,7 +415,7 @@ rocksdb::Status ZSet::RangeByScore(const Slice &user_key, ZRangeSpec spec, std:: return rocksdb::Status::OK(); } -rocksdb::Status ZSet::RangeByLex(const Slice &user_key, ZRangeLexSpec spec, std::vector *members, +rocksdb::Status ZSet::RangeByLex(const Slice &user_key, const ZRangeLexSpec &spec, std::vector *members, int *size) { if (size) *size = 0; if (members) members->clear(); diff --git a/src/types/redis_zset.h b/src/types/redis_zset.h index b97e98be24f..d16999975dc 100644 --- a/src/types/redis_zset.h +++ b/src/types/redis_zset.h @@ -121,7 +121,8 @@ class ZSet : public SubKeyScanner { rocksdb::Status IncrBy(const Slice &user_key, const Slice &member, double increment, double *score); rocksdb::Status Range(const Slice &user_key, int start, int stop, uint8_t flags, std::vector *mscores); rocksdb::Status RangeByScore(const Slice &user_key, ZRangeSpec spec, std::vector *mscores, int *size); - rocksdb::Status RangeByLex(const Slice &user_key, ZRangeLexSpec spec, std::vector *members, int *size); + rocksdb::Status RangeByLex(const Slice &user_key, const ZRangeLexSpec &spec, std::vector *members, + int *size); rocksdb::Status Rank(const Slice &user_key, const Slice &member, bool reversed, int *ret); rocksdb::Status Remove(const Slice &user_key, const std::vector &members, int *ret); rocksdb::Status RemoveRangeByScore(const Slice &user_key, ZRangeSpec spec, int *ret); From afc0291877e7d22ec37d67ba0d432649d87c6918 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Sun, 13 Nov 2022 01:17:55 +0800 Subject: [PATCH 2/3] delete newline --- .clang-tidy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index dcc013c7db6..89be1e4d44d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,8 +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-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-unnecessary-value-param CheckOptions: - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor From 6cd24c64ec0f9c81a3028fe508f13574fe349a2e Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Sun, 13 Nov 2022 01:30:01 +0800 Subject: [PATCH 3/3] fix --- src/server/redis_connection.cc | 2 +- src/server/redis_connection.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index 5cd444a3347..1c4b02f0f96 100644 --- a/src/server/redis_connection.cc +++ b/src/server/redis_connection.cc @@ -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; diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index e6cb7e08d1b..d07c26fbfd3 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -63,7 +63,7 @@ class Connection { 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();