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 some types of warnings #355

Merged
merged 1 commit into from
May 2, 2022
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
6 changes: 4 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ if(REDIS_PLUS_PLUS_BUILD_STATIC)
set_target_properties(${STATIC_LIB} PROPERTIES CXX_STANDARD ${REDIS_PLUS_PLUS_CXX_STANDARD})
set_target_properties(${STATIC_LIB} PROPERTIES OUTPUT_NAME redis++_static)
else()
target_compile_options(${STATIC_LIB} PRIVATE "-Wall" "-W" "-Werror")
# Add -Wsign-conversion when hiredis approves https://github.com/redis/hiredis/pull/1064
target_compile_options(${STATIC_LIB} PRIVATE "-Wall" "-Wextra" "-Wshadow" "-Wconversion" "-Wcast-qual" "-Werror")
set_target_properties(${STATIC_LIB} PROPERTIES OUTPUT_NAME redis++)
endif()

Expand Down Expand Up @@ -222,7 +223,8 @@ if(REDIS_PLUS_PLUS_BUILD_SHARED)
set_target_properties(${SHARED_LIB} PROPERTIES CXX_STANDARD ${REDIS_PLUS_PLUS_CXX_STANDARD})
set_target_properties(${SHARED_LIB} PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE)
else()
target_compile_options(${SHARED_LIB} PRIVATE "-Wall" "-W" "-Werror")
# Add -Wsign-conversion when hiredis approves https://github.com/redis/hiredis/pull/1064
target_compile_options(${SHARED_LIB} PRIVATE "-Wall" "-Wextra" "-Wshadow" "-Wconversion" "-Wcast-qual" "-Werror")
endif()

set_target_properties(${SHARED_LIB} PROPERTIES OUTPUT_NAME redis++)
Expand Down
56 changes: 28 additions & 28 deletions src/sw/redis++/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,29 @@ ConnectionOptions::ConnectionOptions(const std::string &uri) :
ConnectionOptions(_parse_uri(uri)) {}

ConnectionOptions ConnectionOptions::_parse_uri(const std::string &uri) const {
std::string type;
std::string scheme;
std::string auth;
std::string path;
std::tie(type, auth, path) = _split_uri(uri);
std::string spath;
std::tie(scheme, auth, spath) = _split_uri(uri);

ConnectionOptions opts;

_set_auth_opts(auth, opts);

auto db = 0;
auto db_num = 0;
std::string parameter_string;
std::tie(path, db, parameter_string) = _split_path(path);
std::tie(spath, db_num, parameter_string) = _split_path(spath);

_parse_parameters(parameter_string, opts);

opts.db = db;
opts.db = db_num;

if (type == "tcp") {
_set_tcp_opts(path, opts);
} else if (type == "unix") {
_set_unix_opts(path, opts);
if (scheme == "tcp") {
_set_tcp_opts(spath, opts);
} else if (scheme == "unix") {
_set_unix_opts(spath, opts);
} else {
throw Error("invalid URI: invalid type");
throw Error("invalid URI: invalid scheme");
}

return opts;
Expand Down Expand Up @@ -167,42 +167,42 @@ auto ConnectionOptions::_split_uri(const std::string &uri) const
throw Error("invalid URI: no scheme");
}

auto type = uri.substr(0, pos);
auto scheme = uri.substr(0, pos);

auto start = pos + 3;
pos = uri.find("@", start);
if (pos == std::string::npos) {
// No auth info.
return std::make_tuple(type, std::string{}, uri.substr(start));
return std::make_tuple(scheme, std::string{}, uri.substr(start));
}

auto auth = uri.substr(start, pos - start);

return std::make_tuple(type, auth, uri.substr(pos + 1));
return std::make_tuple(scheme, auth, uri.substr(pos + 1));
}

auto ConnectionOptions::_split_path(const std::string &path) const
auto ConnectionOptions::_split_path(const std::string &spath) const
-> std::tuple<std::string, int, std::string> {
auto parameter_pos = path.rfind("?");
auto parameter_pos = spath.rfind("?");
std::string parameter_string;
if (parameter_pos != std::string::npos) {
parameter_string = path.substr(parameter_pos + 1);
parameter_string = spath.substr(parameter_pos + 1);
}

auto pos = path.rfind("/");
auto pos = spath.rfind("/");
if (pos != std::string::npos) {
// Might specified a db number.
try {
auto db = std::stoi(path.substr(pos + 1));
auto db_num = std::stoi(spath.substr(pos + 1));

return std::make_tuple(path.substr(0, pos), db, parameter_string);
return std::make_tuple(spath.substr(0, pos), db_num, parameter_string);
} catch (const std::exception &) {
// Not a db number, and it might be a path to unix domain socket.
}
}

// No db number specified, and use default one, i.e. 0.
return std::make_tuple(path.substr(0, parameter_pos), 0, parameter_string);
return std::make_tuple(spath.substr(0, parameter_pos), 0, parameter_string);
}

void ConnectionOptions::_set_auth_opts(const std::string &auth, ConnectionOptions &opts) const {
Expand All @@ -221,25 +221,25 @@ void ConnectionOptions::_set_auth_opts(const std::string &auth, ConnectionOption
}
}

void ConnectionOptions::_set_tcp_opts(const std::string &path, ConnectionOptions &opts) const {
void ConnectionOptions::_set_tcp_opts(const std::string &spath, ConnectionOptions &opts) const {
opts.type = ConnectionType::TCP;

auto pos = path.find(":");
auto pos = spath.find(":");
if (pos != std::string::npos) {
// Port number specified.
try {
opts.port = std::stoi(path.substr(pos + 1));
opts.port = std::stoi(spath.substr(pos + 1));
} catch (const std::exception &) {
throw Error("invalid URI: invalid port");
}
} // else use default port, i.e. 6379.

opts.host = path.substr(0, pos);
opts.host = spath.substr(0, pos);
}

void ConnectionOptions::_set_unix_opts(const std::string &path, ConnectionOptions &opts) const {
void ConnectionOptions::_set_unix_opts(const std::string &spath, ConnectionOptions &opts) const {
opts.type = ConnectionType::UNIX;
opts.path = path;
opts.path = spath;
}

class Connection::Connector {
Expand Down Expand Up @@ -404,7 +404,7 @@ void Connection::send(CmdArgs &args) {
assert(ctx != nullptr);

if (redisAppendCommandArgv(ctx,
args.size(),
static_cast<int>(args.size()),
args.argv(),
args.argv_len()) != REDIS_OK) {
throw_error(*ctx, "Failed to send command");
Expand Down
5 changes: 2 additions & 3 deletions src/sw/redis++/crc16.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,9 @@ static const uint16_t crc16tab[256]= {
};

uint16_t crc16(const char *buf, int len) {
int counter;
uint16_t crc = 0;
for (counter = 0; counter < len; counter++)
crc = (crc<<8) ^ crc16tab[((crc>>8) ^ *buf++)&0x00FF];
for (int counter = 0; counter < len; counter++)
crc = static_cast<uint16_t>((crc<<8) ^ crc16tab[((crc>>8) ^ *buf++)&0x00FF]);
return crc;
}

Expand Down
12 changes: 6 additions & 6 deletions src/sw/redis++/queued_redis.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ auto QueuedRedis<Impl>::command(Cmd cmd, Args &&...args)
template <typename Impl>
template <typename ...Args>
QueuedRedis<Impl>& QueuedRedis<Impl>::command(const StringView &cmd_name, Args &&...args) {
auto cmd = [](Connection &connection, const StringView &cmd_name, Args &&...args) {
auto cmd = [](Connection &connection, const StringView &name, Args &&...params) {
CmdArgs cmd_args;
cmd_args.append(cmd_name, std::forward<Args>(args)...);
cmd_args.append(name, std::forward<Args>(params)...);
connection.send(cmd_args);
};

Expand All @@ -95,11 +95,11 @@ auto QueuedRedis<Impl>::command(Input first, Input last)
throw Error("command: empty range");
}

auto cmd = [](Connection &connection, Input first, Input last) {
auto cmd = [](Connection &connection, Input start, Input stop) {
CmdArgs cmd_args;
while (first != last) {
cmd_args.append(*first);
++first;
while (start != stop) {
cmd_args.append(*start);
++start;
}
connection.send(cmd_args);
};
Expand Down
12 changes: 6 additions & 6 deletions src/sw/redis++/redis.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ auto Redis::command(Cmd cmd, Args &&...args)
template <typename ...Args>
auto Redis::command(const StringView &cmd_name, Args &&...args)
-> typename std::enable_if<!IsIter<typename LastType<Args...>::type>::value, ReplyUPtr>::type {
auto cmd = [](Connection &connection, const StringView &cmd_name, Args &&...args) {
auto cmd = [](Connection &connection, const StringView &name, Args &&...params) {
CmdArgs cmd_args;
cmd_args.append(cmd_name, std::forward<Args>(args)...);
cmd_args.append(name, std::forward<Args>(params)...);
connection.send(cmd_args);
};

Expand All @@ -65,11 +65,11 @@ auto Redis::command(Input first, Input last)
-> typename std::enable_if<IsIter<Input>::value, ReplyUPtr>::type {
range_check("command", first, last);

auto cmd = [](Connection &connection, Input first, Input last) {
auto cmd = [](Connection &connection, Input start, Input stop) {
CmdArgs cmd_args;
while (first != last) {
cmd_args.append(*first);
++first;
while (start != stop) {
cmd_args.append(*start);
++start;
}
connection.send(cmd_args);
};
Expand Down
16 changes: 8 additions & 8 deletions src/sw/redis++/redis_cluster.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ auto RedisCluster::command(Input first, Input last)
const auto &key = *first;
++first;

auto cmd = [&key](Connection &connection, Input first, Input last) {
auto cmd = [&key](Connection &connection, Input start, Input stop) {
CmdArgs cmd_args;
cmd_args.append(key);
while (first != last) {
cmd_args.append(*first);
++first;
while (start != stop) {
cmd_args.append(*start);
++start;
}
connection.send(cmd_args);
};
Expand Down Expand Up @@ -1337,20 +1337,20 @@ ReplyUPtr RedisCluster::_command(Cmd cmd, const StringView &key, Args &&...args)
SafeConnection safe_connection(*pool);

return _command(cmd, safe_connection.connection(), std::forward<Args>(args)...);
} catch (const IoError &err) {
} catch (const IoError &) {
// When master is down, one of its replicas will be promoted to be the new master.
// If we try to send command to the old master, we'll get an *IoError*.
// In this case, we need to update the slots mapping.
_pool.update();
} catch (const ClosedError &err) {
} catch (const ClosedError &) {
// Node might be removed.
// 1. Get up-to-date slot mapping to check if the node still exists.
_pool.update();

// TODO:
// 2. If it's NOT exist, update slot mapping, and retry.
// 3. If it's still exist, that means the node is down, NOT removed, throw exception.
} catch (const MovedError &err) {
} catch (const MovedError &) {
// Slot mapping has been changed, update it and try again.
_pool.update();
} catch (const AskError &err) {
Expand All @@ -1365,7 +1365,7 @@ ReplyUPtr RedisCluster::_command(Cmd cmd, const StringView &key, Args &&...args)
// 2. resend last command.
try {
return _command(cmd, connection, std::forward<Args>(args)...);
} catch (const MovedError &err) {
} catch (const MovedError &) {
throw Error("Slot migrating... ASKING node hasn't been set to IMPORTING state");
}
} // For other exceptions, just throw it.
Expand Down
8 changes: 4 additions & 4 deletions src/sw/redis++/shards_pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ Node ShardsPool::_parse_node(redisReply *reply) const {
}

auto host = reply::parse<std::string>(*(reply->element[0]));
int port = reply::parse<long long>(*(reply->element[1]));
auto port = static_cast<int>(reply::parse<long long>(*(reply->element[1])));

return {host, port};
}
Expand Down Expand Up @@ -282,11 +282,11 @@ Slot ShardsPool::_slot(const StringView &key) const {
// And I did some minor changes.

const auto *k = key.data();
auto keylen = key.size();
auto keylen = static_cast<int>(key.size());

// start-end indexes of { and }.
std::size_t s = 0;
std::size_t e = 0;
int s = 0;
int e = 0;

// Search the first occurrence of '{'.
for (s = 0; s < keylen; s++)
Expand Down