Skip to content

Commit f58f50e

Browse files
TedLyngmoTedLyngmo
committed
Fix some types of warnings
Also added a few warning options to the build: -Wshadow -Wconversion -Wcast-qual -Wsign-conversion can also be added if the below pull request is approved: redis/hiredis#1064 Signed-off-by: Ted Lyngmo <[email protected]>
1 parent 463f341 commit f58f50e

File tree

7 files changed

+58
-57
lines changed

7 files changed

+58
-57
lines changed

‎CMakeLists.txt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ if(REDIS_PLUS_PLUS_BUILD_STATIC)
167167
set_target_properties(${STATIC_LIB} PROPERTIES CXX_STANDARD ${REDIS_PLUS_PLUS_CXX_STANDARD})
168168
set_target_properties(${STATIC_LIB} PROPERTIES OUTPUT_NAME redis++_static)
169169
else()
170-
target_compile_options(${STATIC_LIB} PRIVATE "-Wall" "-W" "-Werror")
170+
# Add -Wsign-conversion when hiredis approves https://github.com/redis/hiredis/pull/1064
171+
target_compile_options(${STATIC_LIB} PRIVATE "-Wall" "-Wextra" "-Wshadow" "-Wconversion" "-Wcast-qual" "-Werror")
171172
set_target_properties(${STATIC_LIB} PROPERTIES OUTPUT_NAME redis++)
172173
endif()
173174

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

228230
set_target_properties(${SHARED_LIB} PROPERTIES OUTPUT_NAME redis++)

‎src/sw/redis++/connection.cpp‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,29 +36,29 @@ ConnectionOptions::ConnectionOptions(const std::string &uri) :
3636
ConnectionOptions(_parse_uri(uri)) {}
3737

3838
ConnectionOptions ConnectionOptions::_parse_uri(const std::string &uri) const {
39-
std::string type;
39+
std::string scheme;
4040
std::string auth;
41-
std::string path;
42-
std::tie(type, auth, path) = _split_uri(uri);
41+
std::string spath;
42+
std::tie(scheme, auth, spath) = _split_uri(uri);
4343

4444
ConnectionOptions opts;
4545

4646
_set_auth_opts(auth, opts);
4747

48-
auto db = 0;
48+
auto db_num = 0;
4949
std::string parameter_string;
50-
std::tie(path, db, parameter_string) = _split_path(path);
50+
std::tie(spath, db_num, parameter_string) = _split_path(spath);
5151

5252
_parse_parameters(parameter_string, opts);
5353

54-
opts.db = db;
54+
opts.db = db_num;
5555

56-
if (type == "tcp") {
57-
_set_tcp_opts(path, opts);
58-
} else if (type == "unix") {
59-
_set_unix_opts(path, opts);
56+
if (scheme == "tcp") {
57+
_set_tcp_opts(spath, opts);
58+
} else if (scheme == "unix") {
59+
_set_unix_opts(spath, opts);
6060
} else {
61-
throw Error("invalid URI: invalid type");
61+
throw Error("invalid URI: invalid scheme");
6262
}
6363

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

170-
auto type = uri.substr(0, pos);
170+
auto scheme = uri.substr(0, pos);
171171

172172
auto start = pos + 3;
173173
pos = uri.find("@", start);
174174
if (pos == std::string::npos) {
175175
// No auth info.
176-
return std::make_tuple(type, std::string{}, uri.substr(start));
176+
return std::make_tuple(scheme, std::string{}, uri.substr(start));
177177
}
178178

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

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

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

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

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

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

208208
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
221221
}
222222
}
223223

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

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

237-
opts.host = path.substr(0, pos);
237+
opts.host = spath.substr(0, pos);
238238
}
239239

240-
void ConnectionOptions::_set_unix_opts(const std::string &path, ConnectionOptions &opts) const {
240+
void ConnectionOptions::_set_unix_opts(const std::string &spath, ConnectionOptions &opts) const {
241241
opts.type = ConnectionType::UNIX;
242-
opts.path = path;
242+
opts.path = spath;
243243
}
244244

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

406406
if (redisAppendCommandArgv(ctx,
407-
args.size(),
407+
static_cast<int>(args.size()),
408408
args.argv(),
409409
args.argv_len()) != REDIS_OK) {
410410
throw_error(*ctx, "Failed to send command");

‎src/sw/redis++/crc16.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,9 @@ static const uint16_t crc16tab[256]= {
8484
};
8585

8686
uint16_t crc16(const char *buf, int len) {
87-
int counter;
8887
uint16_t crc = 0;
89-
for (counter = 0; counter < len; counter++)
90-
crc = (crc<<8) ^ crc16tab[((crc>>8) ^ *buf++)&0x00FF];
88+
for (int counter = 0; counter < len; counter++)
89+
crc = static_cast<uint16_t>((crc<<8) ^ crc16tab[((crc>>8) ^ *buf++)&0x00FF]);
9190
return crc;
9291
}
9392

‎src/sw/redis++/queued_redis.hpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ auto QueuedRedis<Impl>::command(Cmd cmd, Args &&...args)
7878
template <typename Impl>
7979
template <typename ...Args>
8080
QueuedRedis<Impl>& QueuedRedis<Impl>::command(const StringView &cmd_name, Args &&...args) {
81-
auto cmd = [](Connection &connection, const StringView &cmd_name, Args &&...args) {
81+
auto cmd = [](Connection &connection, const StringView &name, Args &&...params) {
8282
CmdArgs cmd_args;
83-
cmd_args.append(cmd_name, std::forward<Args>(args)...);
83+
cmd_args.append(name, std::forward<Args>(params)...);
8484
connection.send(cmd_args);
8585
};
8686

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

98-
auto cmd = [](Connection &connection, Input first, Input last) {
98+
auto cmd = [](Connection &connection, Input start, Input stop) {
9999
CmdArgs cmd_args;
100-
while (first != last) {
101-
cmd_args.append(*first);
102-
++first;
100+
while (start != stop) {
101+
cmd_args.append(*start);
102+
++start;
103103
}
104104
connection.send(cmd_args);
105105
};

‎src/sw/redis++/redis.hpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ auto Redis::command(Cmd cmd, Args &&...args)
5151
template <typename ...Args>
5252
auto Redis::command(const StringView &cmd_name, Args &&...args)
5353
-> typename std::enable_if<!IsIter<typename LastType<Args...>::type>::value, ReplyUPtr>::type {
54-
auto cmd = [](Connection &connection, const StringView &cmd_name, Args &&...args) {
54+
auto cmd = [](Connection &connection, const StringView &name, Args &&...params) {
5555
CmdArgs cmd_args;
56-
cmd_args.append(cmd_name, std::forward<Args>(args)...);
56+
cmd_args.append(name, std::forward<Args>(params)...);
5757
connection.send(cmd_args);
5858
};
5959

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

68-
auto cmd = [](Connection &connection, Input first, Input last) {
68+
auto cmd = [](Connection &connection, Input start, Input stop) {
6969
CmdArgs cmd_args;
70-
while (first != last) {
71-
cmd_args.append(*first);
72-
++first;
70+
while (start != stop) {
71+
cmd_args.append(*start);
72+
++start;
7373
}
7474
connection.send(cmd_args);
7575
};

‎src/sw/redis++/redis_cluster.hpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ auto RedisCluster::command(Input first, Input last)
8383
const auto &key = *first;
8484
++first;
8585

86-
auto cmd = [&key](Connection &connection, Input first, Input last) {
86+
auto cmd = [&key](Connection &connection, Input start, Input stop) {
8787
CmdArgs cmd_args;
8888
cmd_args.append(key);
89-
while (first != last) {
90-
cmd_args.append(*first);
91-
++first;
89+
while (start != stop) {
90+
cmd_args.append(*start);
91+
++start;
9292
}
9393
connection.send(cmd_args);
9494
};
@@ -1337,20 +1337,20 @@ ReplyUPtr RedisCluster::_command(Cmd cmd, const StringView &key, Args &&...args)
13371337
SafeConnection safe_connection(*pool);
13381338

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

13501350
// TODO:
13511351
// 2. If it's NOT exist, update slot mapping, and retry.
13521352
// 3. If it's still exist, that means the node is down, NOT removed, throw exception.
1353-
} catch (const MovedError &err) {
1353+
} catch (const MovedError &) {
13541354
// Slot mapping has been changed, update it and try again.
13551355
_pool.update();
13561356
} catch (const AskError &err) {
@@ -1365,7 +1365,7 @@ ReplyUPtr RedisCluster::_command(Cmd cmd, const StringView &key, Args &&...args)
13651365
// 2. resend last command.
13661366
try {
13671367
return _command(cmd, connection, std::forward<Args>(args)...);
1368-
} catch (const MovedError &err) {
1368+
} catch (const MovedError &) {
13691369
throw Error("Slot migrating... ASKING node hasn't been set to IMPORTING state");
13701370
}
13711371
} // For other exceptions, just throw it.

‎src/sw/redis++/shards_pool.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ Node ShardsPool::_parse_node(redisReply *reply) const {
234234
}
235235

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

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

284284
const auto *k = key.data();
285-
auto keylen = key.size();
285+
auto keylen = static_cast<int>(key.size());
286286

287287
// start-end indexes of { and }.
288-
std::size_t s = 0;
289-
std::size_t e = 0;
288+
int s = 0;
289+
int e = 0;
290290

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

0 commit comments

Comments
 (0)