Skip to content

Commit be425ed

Browse files
authored
[redisCommand]: Not store the error return code of redisFormat (sonic-net#809)
In the previous implementation, the private member variable, len, will store the error code of redisFormat. This PR is for fixing that if the redisFormat raise an unexpected error code, the `len` will be assigned to zero.
1 parent 5966d8b commit be425ed

3 files changed

Lines changed: 25 additions & 6 deletions

File tree

common/rediscommand.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,18 @@ void RedisCommand::format(const char *fmt, ...)
2525
redisFreeCommand(temp);
2626
temp = nullptr;
2727
}
28+
len = 0;
2829

2930
va_list ap;
3031
va_start(ap, fmt);
31-
len = redisvFormatCommand(&temp, fmt, ap);
32+
int ret = redisvFormatCommand(&temp, fmt, ap);
3233
va_end(ap);
33-
if (len == -1) {
34+
if (ret == -1) {
3435
throw std::bad_alloc();
35-
} else if (len == -2) {
36+
} else if (ret == -2) {
3637
throw std::invalid_argument("fmt");
3738
}
39+
len = ret;
3840
}
3941

4042
void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen)
@@ -44,11 +46,13 @@ void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen
4446
redisFreeCommand(temp);
4547
temp = nullptr;
4648
}
49+
len = 0;
4750

48-
len = redisFormatCommandArgv(&temp, argc, argv, argvlen);
49-
if (len == -1) {
51+
int ret = redisFormatCommandArgv(&temp, argc, argv, argvlen);
52+
if (ret == -1) {
5053
throw std::bad_alloc();
5154
}
55+
len = ret;
5256
}
5357

5458
void RedisCommand::format(const vector<string> &commands)
@@ -135,6 +139,8 @@ std::string RedisCommand::toPrintableString() const
135139

136140
const char *RedisCommand::c_str() const
137141
{
142+
if (len == 0)
143+
return nullptr;
138144
return temp;
139145
}
140146

tests/Makefile.am

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main -lgmock -lgmock_main
55

66
tests_tests_SOURCES = tests/redis_ut.cpp \
77
tests/redis_piped_ut.cpp \
8+
tests/redis_command_ut.cpp \
89
tests/redis_state_ut.cpp \
910
tests/redis_piped_state_ut.cpp \
1011
tests/tokenize_ut.cpp \
@@ -44,5 +45,5 @@ tests_tests_SOURCES = tests/redis_ut.cpp \
4445
tests/main.cpp
4546

4647
tests_tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)
47-
tests_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)
48+
tests_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) -fno-access-control
4849
tests_tests_LDADD = $(LDADD_GTEST) -lpthread common/libswsscommon.la $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) sonic-db-cli/libsonicdbcli.la -lzmq -luuid -lboost_serialization

tests/redis_command_ut.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include "gtest/gtest.h"
2+
#include "common/rediscommand.h"
3+
4+
TEST(RedisCommand, invalid_redis_command)
5+
{
6+
swss::RedisCommand cmd;
7+
EXPECT_THROW(cmd.format("Invalid redis command %l^", 1), std::invalid_argument);
8+
EXPECT_EQ(cmd.c_str(), nullptr);
9+
EXPECT_EQ(cmd.length(), 0);
10+
EXPECT_EQ(cmd.len, 0);
11+
EXPECT_EQ(cmd.temp, nullptr);
12+
}

0 commit comments

Comments
 (0)