Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance-unnecessary-value-param warning reported by clang-tidy #1106

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
@@ -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
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
Expand Down
4 changes: 2 additions & 2 deletions src/commands/redis_cmd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/server/redis_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/server/redis_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions src/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down
7 changes: 4 additions & 3 deletions src/server/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/server/worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_) {
Expand Down
3 changes: 2 additions & 1 deletion src/server/worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/storage/redis_db.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/storage/redis_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/types/redis_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ rocksdb::Status List::PushX(const Slice &user_key, const std::vector<Slice> &ele
return push(user_key, elems, false, left, ret);
}

rocksdb::Status List::push(const Slice &user_key, std::vector<Slice> elems, bool create_if_missing, bool left,
rocksdb::Status List::push(const Slice &user_key, const std::vector<Slice> &elems, bool create_if_missing, bool left,
int *ret) {
*ret = 0;
std::string ns_key;
Expand Down
3 changes: 2 additions & 1 deletion src/types/redis_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Slice> elems, bool create_if_missing, bool left, int *ret);
rocksdb::Status push(const Slice &user_key, const std::vector<Slice> &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);
};
Expand Down
6 changes: 3 additions & 3 deletions src/types/redis_sortedint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint64_t> ids, int *ret) {
rocksdb::Status Sortedint::Add(const Slice &user_key, const std::vector<uint64_t> &ids, int *ret) {
*ret = 0;

std::string ns_key;
Expand Down Expand Up @@ -67,7 +67,7 @@ rocksdb::Status Sortedint::Add(const Slice &user_key, std::vector<uint64_t> ids,
return storage_->Write(storage_->DefaultWriteOptions(), &batch);
}

rocksdb::Status Sortedint::Remove(const Slice &user_key, std::vector<uint64_t> ids, int *ret) {
rocksdb::Status Sortedint::Remove(const Slice &user_key, const std::vector<uint64_t> &ids, int *ret) {
*ret = 0;

std::string ns_key;
Expand Down Expand Up @@ -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<uint64_t> ids, std::vector<int> *exists) {
rocksdb::Status Sortedint::MExist(const Slice &user_key, const std::vector<uint64_t> &ids, std::vector<int> *exists) {
std::string ns_key;
AppendNamespacePrefix(user_key, &ns_key);

Expand Down
6 changes: 3 additions & 3 deletions src/types/redis_sortedint.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint64_t> ids, std::vector<int> *exists);
rocksdb::Status Add(const Slice &user_key, std::vector<uint64_t> ids, int *ret);
rocksdb::Status Remove(const Slice &user_key, std::vector<uint64_t> ids, int *ret);
rocksdb::Status MExist(const Slice &user_key, const std::vector<uint64_t> &ids, std::vector<int> *exists);
rocksdb::Status Add(const Slice &user_key, const std::vector<uint64_t> &ids, int *ret);
rocksdb::Status Remove(const Slice &user_key, const std::vector<uint64_t> &ids, int *ret);
rocksdb::Status Range(const Slice &user_key, uint64_t cursor_id, uint64_t page, uint64_t limit, bool reversed,
std::vector<uint64_t> *ids);
rocksdb::Status RangeByValue(const Slice &user_key, SortedintRangeSpec spec, std::vector<uint64_t> *ids, int *size);
Expand Down
2 changes: 1 addition & 1 deletion src/types/redis_zset.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> *members,
rocksdb::Status ZSet::RangeByLex(const Slice &user_key, const ZRangeLexSpec &spec, std::vector<std::string> *members,
int *size) {
if (size) *size = 0;
if (members) members->clear();
Expand Down
3 changes: 2 additions & 1 deletion src/types/redis_zset.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<MemberScore> *mscores);
rocksdb::Status RangeByScore(const Slice &user_key, ZRangeSpec spec, std::vector<MemberScore> *mscores, int *size);
rocksdb::Status RangeByLex(const Slice &user_key, ZRangeLexSpec spec, std::vector<std::string> *members, int *size);
rocksdb::Status RangeByLex(const Slice &user_key, const ZRangeLexSpec &spec, std::vector<std::string> *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<Slice> &members, int *ret);
rocksdb::Status RemoveRangeByScore(const Slice &user_key, ZRangeSpec spec, int *ret);
Expand Down