Skip to content

Commit

Permalink
[redisCommand]: Not store the error return code of redisFormat (#809)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Pterosaur authored Aug 4, 2023
1 parent 5966d8b commit be425ed
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 6 deletions.
16 changes: 11 additions & 5 deletions common/rediscommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,18 @@ void RedisCommand::format(const char *fmt, ...)
redisFreeCommand(temp);
temp = nullptr;
}
len = 0;

va_list ap;
va_start(ap, fmt);
len = redisvFormatCommand(&temp, fmt, ap);
int ret = redisvFormatCommand(&temp, fmt, ap);
va_end(ap);
if (len == -1) {
if (ret == -1) {
throw std::bad_alloc();
} else if (len == -2) {
} else if (ret == -2) {
throw std::invalid_argument("fmt");
}
len = ret;
}

void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen)
Expand All @@ -44,11 +46,13 @@ void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen
redisFreeCommand(temp);
temp = nullptr;
}
len = 0;

len = redisFormatCommandArgv(&temp, argc, argv, argvlen);
if (len == -1) {
int ret = redisFormatCommandArgv(&temp, argc, argv, argvlen);
if (ret == -1) {
throw std::bad_alloc();
}
len = ret;
}

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

const char *RedisCommand::c_str() const
{
if (len == 0)
return nullptr;
return temp;
}

Expand Down
3 changes: 2 additions & 1 deletion tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main -lgmock -lgmock_main

tests_tests_SOURCES = tests/redis_ut.cpp \
tests/redis_piped_ut.cpp \
tests/redis_command_ut.cpp \
tests/redis_state_ut.cpp \
tests/redis_piped_state_ut.cpp \
tests/tokenize_ut.cpp \
Expand Down Expand Up @@ -44,5 +45,5 @@ tests_tests_SOURCES = tests/redis_ut.cpp \
tests/main.cpp

tests_tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)
tests_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)
tests_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) -fno-access-control
tests_tests_LDADD = $(LDADD_GTEST) -lpthread common/libswsscommon.la $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) sonic-db-cli/libsonicdbcli.la -lzmq -luuid -lboost_serialization
12 changes: 12 additions & 0 deletions tests/redis_command_ut.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include "gtest/gtest.h"
#include "common/rediscommand.h"

TEST(RedisCommand, invalid_redis_command)
{
swss::RedisCommand cmd;
EXPECT_THROW(cmd.format("Invalid redis command %l^", 1), std::invalid_argument);
EXPECT_EQ(cmd.c_str(), nullptr);
EXPECT_EQ(cmd.length(), 0);
EXPECT_EQ(cmd.len, 0);
EXPECT_EQ(cmd.temp, nullptr);
}

0 comments on commit be425ed

Please sign in to comment.