diff --git a/CMakeLists.txt b/CMakeLists.txt index a30558a2..64ab610e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -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() @@ -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++) diff --git a/src/sw/redis++/connection.cpp b/src/sw/redis++/connection.cpp index f0a80882..e1084cea 100644 --- a/src/sw/redis++/connection.cpp +++ b/src/sw/redis++/connection.cpp @@ -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; @@ -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 { - 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 { @@ -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 { @@ -404,7 +404,7 @@ void Connection::send(CmdArgs &args) { assert(ctx != nullptr); if (redisAppendCommandArgv(ctx, - args.size(), + static_cast(args.size()), args.argv(), args.argv_len()) != REDIS_OK) { throw_error(*ctx, "Failed to send command"); diff --git a/src/sw/redis++/crc16.cpp b/src/sw/redis++/crc16.cpp index c94a08d3..6d85a191 100644 --- a/src/sw/redis++/crc16.cpp +++ b/src/sw/redis++/crc16.cpp @@ -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((crc<<8) ^ crc16tab[((crc>>8) ^ *buf++)&0x00FF]); return crc; } diff --git a/src/sw/redis++/queued_redis.hpp b/src/sw/redis++/queued_redis.hpp index d2865e46..d08b41e1 100644 --- a/src/sw/redis++/queued_redis.hpp +++ b/src/sw/redis++/queued_redis.hpp @@ -78,9 +78,9 @@ auto QueuedRedis::command(Cmd cmd, Args &&...args) template template QueuedRedis& QueuedRedis::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)...); + cmd_args.append(name, std::forward(params)...); connection.send(cmd_args); }; @@ -95,11 +95,11 @@ auto QueuedRedis::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); }; diff --git a/src/sw/redis++/redis.hpp b/src/sw/redis++/redis.hpp index c560ce11..e5d8db6d 100644 --- a/src/sw/redis++/redis.hpp +++ b/src/sw/redis++/redis.hpp @@ -51,9 +51,9 @@ auto Redis::command(Cmd cmd, Args &&...args) template auto Redis::command(const StringView &cmd_name, Args &&...args) -> typename std::enable_if::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)...); + cmd_args.append(name, std::forward(params)...); connection.send(cmd_args); }; @@ -65,11 +65,11 @@ auto Redis::command(Input first, Input last) -> typename std::enable_if::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); }; diff --git a/src/sw/redis++/redis_cluster.hpp b/src/sw/redis++/redis_cluster.hpp index 2153d181..de6c2278 100644 --- a/src/sw/redis++/redis_cluster.hpp +++ b/src/sw/redis++/redis_cluster.hpp @@ -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); }; @@ -1337,12 +1337,12 @@ ReplyUPtr RedisCluster::_command(Cmd cmd, const StringView &key, Args &&...args) SafeConnection safe_connection(*pool); return _command(cmd, safe_connection.connection(), std::forward(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(); @@ -1350,7 +1350,7 @@ ReplyUPtr RedisCluster::_command(Cmd cmd, const StringView &key, Args &&...args) // 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) { @@ -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)...); - } 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. diff --git a/src/sw/redis++/shards_pool.cpp b/src/sw/redis++/shards_pool.cpp index e1223163..2951fd2d 100644 --- a/src/sw/redis++/shards_pool.cpp +++ b/src/sw/redis++/shards_pool.cpp @@ -234,7 +234,7 @@ Node ShardsPool::_parse_node(redisReply *reply) const { } auto host = reply::parse(*(reply->element[0])); - int port = reply::parse(*(reply->element[1])); + auto port = static_cast(reply::parse(*(reply->element[1]))); return {host, port}; } @@ -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(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++)