From 148a8f3627961a35c56bf270b28be60aea4d4e39 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 00:26:51 +0800 Subject: [PATCH 01/11] Replace std::stol with ParseInt --- src/redis_cmd.cc | 170 ++++++++++++++++++----------------- src/redis_hash.cc | 13 ++- src/redis_request.cc | 7 +- src/redis_string.cc | 13 ++- src/util.cc | 25 +++--- tests/cppunit/t_hash_test.cc | 7 +- 6 files changed, 120 insertions(+), 115 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index 8943dd0c7cd..b4fee844320 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -488,16 +488,12 @@ class CommandSet : public Commander { if (ttl_ <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); } else if (opt == "px" && !ttl_ && !last_arg) { int64_t ttl_ms = 0; - try { - std::string s = args_[++i]; - std::string::size_type sz; - ttl_ms = std::stol(s, &sz); - if (sz != s.size()) { - return Status(Status::RedisParseErr, errValueNotInterger); - } - } catch (std::exception &e) { + std::string s = args_[++i]; + auto parseResult = ParseInt(s, /* base= */ 10); + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } + ttl_ms = parseResult.GetValue(); if (ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); if (ttl_ms > 0 && ttl_ms < 1000) { ttl_ = 1; // round up the pttl to second @@ -564,17 +560,17 @@ class CommandSetEX : public Commander { class CommandPSetEX : public Commander { public: Status Parse(const std::vector &args) override { - try { - auto ttl_ms = std::stol(args[2]); - if (ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); - if (ttl_ms > 0 && ttl_ms < 1000) { - ttl_ = 1; - } else { - ttl_ = ttl_ms / 1000; - } - } catch (std::exception &e) { + auto parseResult = ParseInt(args[2], /* base= */ 10); + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } + auto ttl_ms = parseResult.GetValue(); + if (ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); + if (ttl_ms > 0 && ttl_ms < 1000) { + ttl_ = 1; + } else { + ttl_ = ttl_ms / 1000; + } return Commander::Parse(args); } @@ -677,11 +673,11 @@ class CommandDecr : public Commander { class CommandIncrBy : public Commander { public: Status Parse(const std::vector &args) override { - try { - increment_ = std::stoll(args[2]); - } catch (std::exception &e) { + auto parseResult = ParseInt(args[2], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + increment_ = parseResult.GetValue(); return Commander::Parse(args); } @@ -725,11 +721,11 @@ class CommandIncrByFloat : public Commander { class CommandDecrBy : public Commander { public: Status Parse(const std::vector &args) override { - try { - increment_ = std::stoll(args[2]); - } catch (std::exception &e) { + auto parseResult = ParseInt(args[2], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + increment_ = parseResult.GetValue(); return Commander::Parse(args); } @@ -820,12 +816,11 @@ class CommandDel : public Commander { }; Status getBitOffsetFromArgument(std::string arg, uint32_t *offset) { - int64_t offset_arg = 0; - try { - offset_arg = std::stoll(arg); - } catch (std::exception &e) { + auto parseResult = ParseInt(arg, /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + int64_t offset_arg = offset_arg = parseResult.GetValue(); if (offset_arg < 0 || offset_arg > UINT_MAX) { return Status(Status::RedisParseErr, "bit offset is out of range"); } @@ -889,12 +884,16 @@ class CommandBitCount : public Commander { return Status(Status::RedisParseErr, errInvalidSyntax); } if (args.size() == 4) { - try { - start_ = std::stol(args[2]); - stop_ = std::stol(args[3]); - } catch (std::exception &e) { + auto parseArgs2Result = ParseInt(args[2], /* base= */ 10); + if (!parseArgs2Result.IsOK()){ + return Status(Status::RedisParseErr, errValueNotInterger); + } + start_ = parseArgs2Result.GetValue(); + auto parseArgs3Result = ParseInt(args[3], /* base= */ 10); + if (!parseArgs3Result.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + stop_ = parseArgs3Result.GetValue(); } return Commander::Parse(args); } @@ -914,15 +913,21 @@ class CommandBitCount : public Commander { class CommandBitPos: public Commander { public: - Status Parse(const std::vector &args) override { - try { - if (args.size() >= 4) start_ = std::stol(args[3]); - if (args.size() >= 5) { - stop_given_ = true; - stop_ = std::stol(args[4]); + Status Parse(const std::vector &args) override { + if (args.size() >= 4){ + auto parseArgs3Result = ParseInt(args[3], /* base= */ 10); + if (!parseArgs3Result.IsOK()){ + return Status(Status::RedisParseErr, errValueNotInterger); } - } catch (std::exception &e) { - return Status(Status::RedisParseErr, errValueNotInterger); + start_ = parseArgs3Result.GetValue(); + } + if (args.size() >= 5) { + auto parseArgs4Result = ParseInt(args[4], /* base= */ 10); + if (!parseArgs4Result.IsOK()){ + return Status(Status::RedisParseErr, errValueNotInterger); + } + stop_given_ = true; + stop_ = parseArgs4Result.GetValue(); } if (args[2] == "0") { bit_ = false; @@ -1104,20 +1109,20 @@ class CommandPExpire : public Commander { Status Parse(const std::vector &args) override { int64_t now; rocksdb::Env::Default()->GetCurrentTime(&now); - try { - auto ttl_ms = std::stol(args[2]); - if (ttl_ms > 0 && ttl_ms < 1000) { - seconds_ = 1; - } else { - seconds_ = ttl_ms / 1000; - if (seconds_ >= INT32_MAX - now) { - return Status(Status::RedisParseErr, "the expire time was overflow"); - } - } - seconds_ += now; - } catch (std::exception &e) { + auto parseResult = ParseInt(args[2], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + auto ttl_ms = parseResult.GetValue(); + if (ttl_ms > 0 && ttl_ms < 1000) { + seconds_ = 1; + } else { + seconds_ = ttl_ms / 1000; + if (seconds_ >= INT32_MAX - now) { + return Status(Status::RedisParseErr, "the expire time was overflow"); + } + } + seconds_ += now; return Commander::Parse(args); } @@ -1167,14 +1172,14 @@ class CommandExpireAt : public Commander { class CommandPExpireAt : public Commander { public: Status Parse(const std::vector &args) override { - try { - timestamp_ = static_cast(std::stol(args[2])/1000); - if (timestamp_ >= INT32_MAX) { - return Status(Status::RedisParseErr, "the expire time was overflow"); - } - } catch (std::exception &e) { + auto parseResult = ParseInt(args[2], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + if (parseResult.GetValue()/1000 >= INT32_MAX) { + return Status(Status::RedisParseErr, "the expire time was overflow"); + } + timestamp_ = static_cast(parseResult.GetValue()/1000); return Commander::Parse(args); } Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -1301,11 +1306,11 @@ class CommandHLen : public Commander { class CommandHIncrBy : public Commander { public: Status Parse(const std::vector &args) override { - try { - increment_ = std::stoll(args[3]); - } catch (std::exception &e) { + auto parseResult = ParseInt(args[3], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + increment_ = parseResult.GetValue(); return Commander::Parse(args); } Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -1517,16 +1522,16 @@ class CommandPop : public Commander { if (args.size() == 2) { return Status::OK(); } - try { - int32_t v = std::stol(args[2]); - if (v < 0) { - return Status(Status::RedisParseErr, errValueMustBePositive); - } - count_ = v; - with_count_ = true; - } catch (const std::exception& ) { + auto parseResult = ParseInt(args[2], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + int32_t v = parseResult.GetValue(); + if (v < 0) { + return Status(Status::RedisParseErr, errValueMustBePositive); + } + count_ = v; + with_count_ = true; return Status::OK(); } @@ -3962,11 +3967,11 @@ class CommandClient : public Commander { if (!strcasecmp(args[i].c_str(), "addr") && moreargs) { addr_ = args[i+1]; } else if (!strcasecmp(args[i].c_str(), "id") && moreargs) { - try { - id_ = std::stoll(args[i+1]); - } catch (std::exception &e) { + auto parseResult = ParseInt(args[i+1], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + id_ = parseResult.GetValue(); } else if (!strcasecmp(args[i].c_str(), "skipme") && moreargs) { if (!strcasecmp(args[i+1].c_str(), "yes")) { skipme_ = true; @@ -5363,13 +5368,12 @@ class CommandXRead : public Commander { if (i+1 >= args.size()) { return Status(Status::RedisParseErr, errInvalidSyntax); } - - try { - with_count_ = true; - count_ = static_cast(std::stoll(args[i+1])); - } catch (const std::exception &) { + with_count_ = true; + auto parseResult = ParseInt(args[i+1], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + count_ = static_cast(parseResult.GetValue()); i += 2; continue; } @@ -5380,15 +5384,15 @@ class CommandXRead : public Commander { } block_ = true; - try { - auto v = std::stoll(args[i+1]); - if (v < 0) { - return Status(Status::RedisParseErr, errTimeoutIsNegative); - } - block_timeout_ = v; - } catch (const std::exception &) { + auto parseResult = ParseInt(args[i+1], /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::RedisParseErr, errValueNotInterger); } + auto v = parseResult.GetValue(); + if (v < 0) { + return Status(Status::RedisParseErr, errTimeoutIsNegative); + } + block_timeout_ = v; i += 2; continue; } diff --git a/src/redis_hash.cc b/src/redis_hash.cc index 36c1597b4b2..8e32ae32670 100644 --- a/src/redis_hash.cc +++ b/src/redis_hash.cc @@ -25,6 +25,7 @@ #include #include #include "db_util.h" +#include "parse_util.h" namespace Redis { rocksdb::Status Hash::GetMetadata(const Slice &ns_key, HashMetadata *metadata) { @@ -73,18 +74,14 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t InternalKey(ns_key, field, metadata.version, storage_->IsSlotIdEncoded()).Encode(&sub_key); if (s.ok()) { std::string value_bytes; - std::size_t idx = 0; s = db_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes); if (!s.ok() && !s.IsNotFound()) return s; if (s.ok()) { - try { - old_value = std::stoll(value_bytes, &idx); - } catch (std::exception &e) { - return rocksdb::Status::InvalidArgument(e.what()); - } - if (isspace(value_bytes[0]) || idx != value_bytes.size()) { - return rocksdb::Status::InvalidArgument("value is not an integer"); + auto parseResult = ParseInt(value_bytes, /* base= */ 10); + if (!parseResult.IsOK()){ + return rocksdb::Status::InvalidArgument(parseResult.Msg()); } + old_value = parseResult.GetValue(); exists = true; } } diff --git a/src/redis_request.cc b/src/redis_request.cc index 88a18c42a38..1b333fa7359 100644 --- a/src/redis_request.cc +++ b/src/redis_request.cc @@ -32,6 +32,7 @@ #include "server.h" #include "redis_slot.h" #include "event_util.h" +#include "parse_util.h" namespace Redis { const size_t PROTO_INLINE_MAX_SIZE = 16 * 1024L; @@ -66,11 +67,11 @@ Status Request::Tokenize(evbuffer *input) { pipeline_size++; svr_->stats_.IncrInbondBytes(line.length); if (line[0] == '*') { - try { - multi_bulk_len_ = std::stoll(std::string(line.get() + 1, line.length - 1)); - } catch (std::exception &e) { + auto parseResult = ParseInt(std::string(line.get() + 1, line.length - 1), /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::NotOK, "Protocol error: invalid multibulk length"); } + multi_bulk_len_ = parseResult.GetValue(); if (isOnlyLF || multi_bulk_len_ > (int64_t)PROTO_MULTI_MAX_SIZE) { return Status(Status::NotOK, "Protocol error: invalid multibulk length"); } diff --git a/src/redis_string.cc b/src/redis_string.cc index 20ea14c6dc4..b2320a102bc 100644 --- a/src/redis_string.cc +++ b/src/redis_string.cc @@ -23,6 +23,7 @@ #include #include #include +#include "parse_util.h" namespace Redis { @@ -274,16 +275,12 @@ rocksdb::Status String::IncrBy(const std::string &user_key, int64_t increment, i value = raw_value.substr(STRING_HDR_SIZE, raw_value.size()-STRING_HDR_SIZE); int64_t n = 0; - std::size_t idx = 0; if (!value.empty()) { - try { - n = std::stoll(value, &idx); - } catch(std::exception &e) { - return rocksdb::Status::InvalidArgument("value is not an integer or out of range"); - } - if (isspace(value[0]) || idx != value.size()) { - return rocksdb::Status::InvalidArgument("value is not an integer"); + auto parseResult = ParseInt(value, /* base= */ 10); + if (!parseResult.IsOK()){ + return rocksdb::Status::InvalidArgument(parseResult.Msg()); } + n = parseResult.GetValue(); } if ((increment < 0 && n <= 0 && increment < (LLONG_MIN-n)) || (increment > 0 && n >= 0 && increment > (LLONG_MAX-n))) { diff --git a/src/util.cc b/src/util.cc index 31944f027f0..4c20bfd8e8f 100644 --- a/src/util.cc +++ b/src/util.cc @@ -44,6 +44,7 @@ #include "util.h" #include "status.h" #include "event_util.h" +#include "parse_util.h" #ifndef POLLIN # define POLLIN 0x0001 /* There is data to read */ @@ -341,26 +342,26 @@ int GetLocalPort(int fd) { } Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - try { - *n = static_cast(std::stoll(str)); - if (max > min && (*n < min || *n > max)) { - return Status(Status::NotOK, "value should between "+std::to_string(min)+" and "+std::to_string(max)); - } - } catch (std::exception &e) { + auto parseResult = ParseInt(str, /* base= */ 10); + if (!parseResult.IsOK()){ return Status(Status::NotOK, "value is not an integer or out of range"); } + *n = parseResult.GetValue(); + if (max > min && (*n < min || *n > max)) { + return Status(Status::NotOK, "value should between "+std::to_string(min)+" and "+std::to_string(max)); + } return Status::OK(); } Status OctalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - try { - *n = static_cast(std::stoll(str, nullptr, 8)); - if (max > min && (*n < min || *n > max)) { - return Status(Status::NotOK, "value should between "+std::to_string(min)+" and "+std::to_string(max)); - } - } catch (std::exception &e) { + auto parseResult = ParseInt(str, /* base= */ 8); + if (!parseResult.IsOK()){ return Status(Status::NotOK, "value is not an integer or out of range"); } + *n = parseResult.GetValue(); + if (max > min && (*n < min || *n > max)) { + return Status(Status::NotOK, "value should between "+std::to_string(min)+" and "+std::to_string(max)); + } return Status::OK(); } diff --git a/tests/cppunit/t_hash_test.cc b/tests/cppunit/t_hash_test.cc index f1153c61bb2..6743ccaf45b 100644 --- a/tests/cppunit/t_hash_test.cc +++ b/tests/cppunit/t_hash_test.cc @@ -23,6 +23,7 @@ #include "test_base.h" #include "redis_hash.h" +#include "parse_util.h" class RedisHashTest : public TestBase { protected: explicit RedisHashTest() : TestBase() { @@ -114,7 +115,11 @@ TEST_F(RedisHashTest, HIncr) { } std::string bytes; hash->Get(key_, field, &bytes); - value = std::stoll(bytes); + auto parseResult = ParseInt(bytes, /* base= */ 10); + if (!parseResult.IsOK()){ + EXPECT_TRUE(false); + } + value = parseResult.GetValue(); EXPECT_EQ(32, value); hash->Del(key_); } From 7aa679f48e368cd08251936a573c230a50ccb2eb Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 00:32:07 +0800 Subject: [PATCH 02/11] fix cpp link --- src/redis_cmd.cc | 22 +++++++++++----------- src/redis_hash.cc | 2 +- src/redis_request.cc | 2 +- src/redis_string.cc | 2 +- src/util.cc | 4 ++-- tests/cppunit/t_hash_test.cc | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index b4fee844320..735b066acf2 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -674,7 +674,7 @@ class CommandIncrBy : public Commander { public: Status Parse(const std::vector &args) override { auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } increment_ = parseResult.GetValue(); @@ -722,7 +722,7 @@ class CommandDecrBy : public Commander { public: Status Parse(const std::vector &args) override { auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } increment_ = parseResult.GetValue(); @@ -817,7 +817,7 @@ class CommandDel : public Commander { Status getBitOffsetFromArgument(std::string arg, uint32_t *offset) { auto parseResult = ParseInt(arg, /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } int64_t offset_arg = offset_arg = parseResult.GetValue(); @@ -913,7 +913,7 @@ class CommandBitCount : public Commander { class CommandBitPos: public Commander { public: - Status Parse(const std::vector &args) override { + Status Parse(const std::vector &args) override { if (args.size() >= 4){ auto parseArgs3Result = ParseInt(args[3], /* base= */ 10); if (!parseArgs3Result.IsOK()){ @@ -1110,7 +1110,7 @@ class CommandPExpire : public Commander { int64_t now; rocksdb::Env::Default()->GetCurrentTime(&now); auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } auto ttl_ms = parseResult.GetValue(); @@ -1173,7 +1173,7 @@ class CommandPExpireAt : public Commander { public: Status Parse(const std::vector &args) override { auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } if (parseResult.GetValue()/1000 >= INT32_MAX) { @@ -1307,7 +1307,7 @@ class CommandHIncrBy : public Commander { public: Status Parse(const std::vector &args) override { auto parseResult = ParseInt(args[3], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } increment_ = parseResult.GetValue(); @@ -1523,7 +1523,7 @@ class CommandPop : public Commander { return Status::OK(); } auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } int32_t v = parseResult.GetValue(); @@ -3968,7 +3968,7 @@ class CommandClient : public Commander { addr_ = args[i+1]; } else if (!strcasecmp(args[i].c_str(), "id") && moreargs) { auto parseResult = ParseInt(args[i+1], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } id_ = parseResult.GetValue(); @@ -5370,7 +5370,7 @@ class CommandXRead : public Commander { } with_count_ = true; auto parseResult = ParseInt(args[i+1], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } count_ = static_cast(parseResult.GetValue()); @@ -5385,7 +5385,7 @@ class CommandXRead : public Commander { block_ = true; auto parseResult = ParseInt(args[i+1], /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::RedisParseErr, errValueNotInterger); } auto v = parseResult.GetValue(); diff --git a/src/redis_hash.cc b/src/redis_hash.cc index 8e32ae32670..da5313ac67f 100644 --- a/src/redis_hash.cc +++ b/src/redis_hash.cc @@ -78,7 +78,7 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t if (!s.ok() && !s.IsNotFound()) return s; if (s.ok()) { auto parseResult = ParseInt(value_bytes, /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return rocksdb::Status::InvalidArgument(parseResult.Msg()); } old_value = parseResult.GetValue(); diff --git a/src/redis_request.cc b/src/redis_request.cc index 1b333fa7359..ee04f220e56 100644 --- a/src/redis_request.cc +++ b/src/redis_request.cc @@ -68,7 +68,7 @@ Status Request::Tokenize(evbuffer *input) { svr_->stats_.IncrInbondBytes(line.length); if (line[0] == '*') { auto parseResult = ParseInt(std::string(line.get() + 1, line.length - 1), /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::NotOK, "Protocol error: invalid multibulk length"); } multi_bulk_len_ = parseResult.GetValue(); diff --git a/src/redis_string.cc b/src/redis_string.cc index b2320a102bc..0c923279f95 100644 --- a/src/redis_string.cc +++ b/src/redis_string.cc @@ -277,7 +277,7 @@ rocksdb::Status String::IncrBy(const std::string &user_key, int64_t increment, i int64_t n = 0; if (!value.empty()) { auto parseResult = ParseInt(value, /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return rocksdb::Status::InvalidArgument(parseResult.Msg()); } n = parseResult.GetValue(); diff --git a/src/util.cc b/src/util.cc index 4c20bfd8e8f..03962039bcf 100644 --- a/src/util.cc +++ b/src/util.cc @@ -343,7 +343,7 @@ int GetLocalPort(int fd) { Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { auto parseResult = ParseInt(str, /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::NotOK, "value is not an integer or out of range"); } *n = parseResult.GetValue(); @@ -355,7 +355,7 @@ Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64 Status OctalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { auto parseResult = ParseInt(str, /* base= */ 8); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { return Status(Status::NotOK, "value is not an integer or out of range"); } *n = parseResult.GetValue(); diff --git a/tests/cppunit/t_hash_test.cc b/tests/cppunit/t_hash_test.cc index 6743ccaf45b..ea2803673a9 100644 --- a/tests/cppunit/t_hash_test.cc +++ b/tests/cppunit/t_hash_test.cc @@ -116,7 +116,7 @@ TEST_F(RedisHashTest, HIncr) { std::string bytes; hash->Get(key_, field, &bytes); auto parseResult = ParseInt(bytes, /* base= */ 10); - if (!parseResult.IsOK()){ + if (!parseResult.IsOK()) { EXPECT_TRUE(false); } value = parseResult.GetValue(); From e21bd11592d4cb7164d8914a85da7e9525b4c4c1 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 01:07:57 +0800 Subject: [PATCH 03/11] Fix question asked by review --- src/redis_cmd.cc | 107 +++++++++++++++++------------------ src/redis_hash.cc | 6 +- src/redis_request.cc | 6 +- src/redis_string.cc | 6 +- src/util.cc | 22 +++---- tests/cppunit/t_hash_test.cc | 9 ++- 6 files changed, 74 insertions(+), 82 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index 735b066acf2..e2c1a8493bb 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -489,11 +489,11 @@ class CommandSet : public Commander { } else if (opt == "px" && !ttl_ && !last_arg) { int64_t ttl_ms = 0; std::string s = args_[++i]; - auto parseResult = ParseInt(s, /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(s, 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - ttl_ms = parseResult.GetValue(); + ttl_ms = *parseResult; if (ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); if (ttl_ms > 0 && ttl_ms < 1000) { ttl_ = 1; // round up the pttl to second @@ -560,11 +560,11 @@ class CommandSetEX : public Commander { class CommandPSetEX : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[2], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - auto ttl_ms = parseResult.GetValue(); + auto ttl_ms = *parseResult; if (ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); if (ttl_ms > 0 && ttl_ms < 1000) { ttl_ = 1; @@ -673,11 +673,11 @@ class CommandDecr : public Commander { class CommandIncrBy : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[2], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - increment_ = parseResult.GetValue(); + increment_ = *parseResult; return Commander::Parse(args); } @@ -721,11 +721,11 @@ class CommandIncrByFloat : public Commander { class CommandDecrBy : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[2], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - increment_ = parseResult.GetValue(); + increment_ = *parseResult; return Commander::Parse(args); } @@ -816,11 +816,11 @@ class CommandDel : public Commander { }; Status getBitOffsetFromArgument(std::string arg, uint32_t *offset) { - auto parseResult = ParseInt(arg, /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(arg, 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - int64_t offset_arg = offset_arg = parseResult.GetValue(); + int64_t offset_arg = offset_arg = *parseResult; if (offset_arg < 0 || offset_arg > UINT_MAX) { return Status(Status::RedisParseErr, "bit offset is out of range"); } @@ -884,16 +884,16 @@ class CommandBitCount : public Commander { return Status(Status::RedisParseErr, errInvalidSyntax); } if (args.size() == 4) { - auto parseArgs2Result = ParseInt(args[2], /* base= */ 10); - if (!parseArgs2Result.IsOK()){ + auto parseArgs2Result = ParseInt(args[2], 10); + if (!parseArgs2Result) { return Status(Status::RedisParseErr, errValueNotInterger); } - start_ = parseArgs2Result.GetValue(); - auto parseArgs3Result = ParseInt(args[3], /* base= */ 10); - if (!parseArgs3Result.IsOK()){ + start_ = *parseArgs2Result; + auto parseArgs3Result = ParseInt(args[3], 10); + if (!parseArgs3Result) { return Status(Status::RedisParseErr, errValueNotInterger); } - stop_ = parseArgs3Result.GetValue(); + stop_ = *parseArgs3Result; } return Commander::Parse(args); } @@ -915,19 +915,19 @@ class CommandBitPos: public Commander { public: Status Parse(const std::vector &args) override { if (args.size() >= 4){ - auto parseArgs3Result = ParseInt(args[3], /* base= */ 10); - if (!parseArgs3Result.IsOK()){ + auto parseArgs3Result = ParseInt(args[3], 10); + if (!parseArgs3Result){ return Status(Status::RedisParseErr, errValueNotInterger); } - start_ = parseArgs3Result.GetValue(); + start_ = *parseArgs3Result; } if (args.size() >= 5) { - auto parseArgs4Result = ParseInt(args[4], /* base= */ 10); - if (!parseArgs4Result.IsOK()){ + auto parseArgs4Result = ParseInt(args[4], 10); + if (!parseArgs4Result){ return Status(Status::RedisParseErr, errValueNotInterger); } stop_given_ = true; - stop_ = parseArgs4Result.GetValue(); + stop_ = *parseArgs4Result; } if (args[2] == "0") { bit_ = false; @@ -1109,11 +1109,11 @@ class CommandPExpire : public Commander { Status Parse(const std::vector &args) override { int64_t now; rocksdb::Env::Default()->GetCurrentTime(&now); - auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[2], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - auto ttl_ms = parseResult.GetValue(); + auto ttl_ms = *parseResult; if (ttl_ms > 0 && ttl_ms < 1000) { seconds_ = 1; } else { @@ -1172,14 +1172,14 @@ class CommandExpireAt : public Commander { class CommandPExpireAt : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[2], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - if (parseResult.GetValue()/1000 >= INT32_MAX) { + if (*parseResult/1000 >= INT32_MAX) { return Status(Status::RedisParseErr, "the expire time was overflow"); } - timestamp_ = static_cast(parseResult.GetValue()/1000); + timestamp_ = static_cast(*parseResult/1000); return Commander::Parse(args); } Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -1306,11 +1306,11 @@ class CommandHLen : public Commander { class CommandHIncrBy : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[3], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[3], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - increment_ = parseResult.GetValue(); + increment_ = *parseResult; return Commander::Parse(args); } Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -1522,11 +1522,11 @@ class CommandPop : public Commander { if (args.size() == 2) { return Status::OK(); } - auto parseResult = ParseInt(args[2], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[2], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - int32_t v = parseResult.GetValue(); + int32_t v = *parseResult; if (v < 0) { return Status(Status::RedisParseErr, errValueMustBePositive); } @@ -3967,11 +3967,11 @@ class CommandClient : public Commander { if (!strcasecmp(args[i].c_str(), "addr") && moreargs) { addr_ = args[i+1]; } else if (!strcasecmp(args[i].c_str(), "id") && moreargs) { - auto parseResult = ParseInt(args[i+1], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[i+1], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - id_ = parseResult.GetValue(); + id_ = *parseResult; } else if (!strcasecmp(args[i].c_str(), "skipme") && moreargs) { if (!strcasecmp(args[i+1].c_str(), "yes")) { skipme_ = true; @@ -4163,12 +4163,12 @@ class CommandHello final : public Commander { size_t next_arg = 1; if (args_.size() >= 2) { int64_t protocol; - auto parseResult = ParseInt(args_[next_arg], /* base= */ 10); + auto parseResult = ParseInt(args_[next_arg], 10); ++next_arg; - if (!parseResult.IsOK()) { + if (!parseResult) { return Status(Status::NotOK, "Protocol version is not an integer or out of range"); } - protocol = parseResult.GetValue(); + protocol = *parseResult; // In redis, it will check protocol < 2 or protocol > 3, // kvrocks only supports REPL2 by now, but for supporting some @@ -5369,11 +5369,11 @@ class CommandXRead : public Commander { return Status(Status::RedisParseErr, errInvalidSyntax); } with_count_ = true; - auto parseResult = ParseInt(args[i+1], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[i+1], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - count_ = static_cast(parseResult.GetValue()); + count_ = *parseResult; i += 2; continue; } @@ -5384,15 +5384,14 @@ class CommandXRead : public Commander { } block_ = true; - auto parseResult = ParseInt(args[i+1], /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(args[i+1], 10); + if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - auto v = parseResult.GetValue(); - if (v < 0) { + if (*parseResult < 0) { return Status(Status::RedisParseErr, errTimeoutIsNegative); } - block_timeout_ = v; + block_timeout_ = *parseResult; i += 2; continue; } diff --git a/src/redis_hash.cc b/src/redis_hash.cc index da5313ac67f..8514bf47ed9 100644 --- a/src/redis_hash.cc +++ b/src/redis_hash.cc @@ -77,11 +77,11 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t s = db_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes); if (!s.ok() && !s.IsNotFound()) return s; if (s.ok()) { - auto parseResult = ParseInt(value_bytes, /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(value_bytes, 10); + if (!parseResult) { return rocksdb::Status::InvalidArgument(parseResult.Msg()); } - old_value = parseResult.GetValue(); + old_value = *parseResult; exists = true; } } diff --git a/src/redis_request.cc b/src/redis_request.cc index ee04f220e56..9e094ab134b 100644 --- a/src/redis_request.cc +++ b/src/redis_request.cc @@ -67,11 +67,11 @@ Status Request::Tokenize(evbuffer *input) { pipeline_size++; svr_->stats_.IncrInbondBytes(line.length); if (line[0] == '*') { - auto parseResult = ParseInt(std::string(line.get() + 1, line.length - 1), /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(std::string(line.get() + 1, line.length - 1), 10); + if (!parseResult) { return Status(Status::NotOK, "Protocol error: invalid multibulk length"); } - multi_bulk_len_ = parseResult.GetValue(); + multi_bulk_len_ = *parseResult; if (isOnlyLF || multi_bulk_len_ > (int64_t)PROTO_MULTI_MAX_SIZE) { return Status(Status::NotOK, "Protocol error: invalid multibulk length"); } diff --git a/src/redis_string.cc b/src/redis_string.cc index 0c923279f95..0f5a0ac04cb 100644 --- a/src/redis_string.cc +++ b/src/redis_string.cc @@ -276,11 +276,11 @@ rocksdb::Status String::IncrBy(const std::string &user_key, int64_t increment, i value = raw_value.substr(STRING_HDR_SIZE, raw_value.size()-STRING_HDR_SIZE); int64_t n = 0; if (!value.empty()) { - auto parseResult = ParseInt(value, /* base= */ 10); - if (!parseResult.IsOK()) { + auto parseResult = ParseInt(value, 10); + if (!parseResult) { return rocksdb::Status::InvalidArgument(parseResult.Msg()); } - n = parseResult.GetValue(); + n = *parseResult; } if ((increment < 0 && n <= 0 && increment < (LLONG_MIN-n)) || (increment > 0 && n >= 0 && increment > (LLONG_MAX-n))) { diff --git a/src/util.cc b/src/util.cc index 03962039bcf..a1ff4f22524 100644 --- a/src/util.cc +++ b/src/util.cc @@ -342,26 +342,20 @@ int GetLocalPort(int fd) { } Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - auto parseResult = ParseInt(str, /* base= */ 10); - if (!parseResult.IsOK()) { - return Status(Status::NotOK, "value is not an integer or out of range"); - } - *n = parseResult.GetValue(); - if (max > min && (*n < min || *n > max)) { - return Status(Status::NotOK, "value should between "+std::to_string(min)+" and "+std::to_string(max)); + auto parseResult = ParseInt(str, NumericRange{min,max}, 10); + if (!parseResult) { + return Status(Status::NotOK, parseResult.Msg()); } + *n = *parseResult; return Status::OK(); } Status OctalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - auto parseResult = ParseInt(str, /* base= */ 8); - if (!parseResult.IsOK()) { - return Status(Status::NotOK, "value is not an integer or out of range"); - } - *n = parseResult.GetValue(); - if (max > min && (*n < min || *n > max)) { - return Status(Status::NotOK, "value should between "+std::to_string(min)+" and "+std::to_string(max)); + auto parseResult = ParseInt(str, NumericRange{min,max}, 8); + if (!parseResult) { + return Status(Status::NotOK, parseResult.Msg()); } + *n = *parseResult; return Status::OK(); } diff --git a/tests/cppunit/t_hash_test.cc b/tests/cppunit/t_hash_test.cc index ea2803673a9..6c4d86c54d3 100644 --- a/tests/cppunit/t_hash_test.cc +++ b/tests/cppunit/t_hash_test.cc @@ -115,12 +115,11 @@ TEST_F(RedisHashTest, HIncr) { } std::string bytes; hash->Get(key_, field, &bytes); - auto parseResult = ParseInt(bytes, /* base= */ 10); - if (!parseResult.IsOK()) { - EXPECT_TRUE(false); + auto parseResult = ParseInt(bytes, 10); + if (!parseResult) { + FAIL(); } - value = parseResult.GetValue(); - EXPECT_EQ(32, value); + EXPECT_EQ(32, *parseResult); hash->Del(key_); } From a584e01eea27d78beced21fdb9ffb70aa39ee834 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 01:11:46 +0800 Subject: [PATCH 04/11] Fix question asked by review --- src/redis_cmd.cc | 6 +++--- src/util.cc | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index e2c1a8493bb..c13f171945d 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -914,16 +914,16 @@ class CommandBitCount : public Commander { class CommandBitPos: public Commander { public: Status Parse(const std::vector &args) override { - if (args.size() >= 4){ + if (args.size() >= 4) { auto parseArgs3Result = ParseInt(args[3], 10); - if (!parseArgs3Result){ + if (!parseArgs3Result) { return Status(Status::RedisParseErr, errValueNotInterger); } start_ = *parseArgs3Result; } if (args.size() >= 5) { auto parseArgs4Result = ParseInt(args[4], 10); - if (!parseArgs4Result){ + if (!parseArgs4Result) { return Status(Status::RedisParseErr, errValueNotInterger); } stop_given_ = true; diff --git a/src/util.cc b/src/util.cc index a1ff4f22524..a44c9b29bc3 100644 --- a/src/util.cc +++ b/src/util.cc @@ -342,7 +342,7 @@ int GetLocalPort(int fd) { } Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - auto parseResult = ParseInt(str, NumericRange{min,max}, 10); + auto parseResult = ParseInt(str, NumericRange{min, max}, 10); if (!parseResult) { return Status(Status::NotOK, parseResult.Msg()); } @@ -351,7 +351,7 @@ Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64 } Status OctalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - auto parseResult = ParseInt(str, NumericRange{min,max}, 8); + auto parseResult = ParseInt(str, NumericRange{min, max}, 8); if (!parseResult) { return Status(Status::NotOK, parseResult.Msg()); } From fb1b41e6e8c9a1e7eb951c476644be87fc4dcbbf Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 01:18:57 +0800 Subject: [PATCH 05/11] fix typo --- src/redis_cmd.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index c13f171945d..ff6f3a1f433 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -820,7 +820,7 @@ Status getBitOffsetFromArgument(std::string arg, uint32_t *offset) { if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } - int64_t offset_arg = offset_arg = *parseResult; + int64_t offset_arg = *parseResult; if (offset_arg < 0 || offset_arg > UINT_MAX) { return Status(Status::RedisParseErr, "bit offset is out of range"); } From 6903f9103cf31f325452b453ad389df65ccd21e5 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 01:33:26 +0800 Subject: [PATCH 06/11] fix type typo --- src/redis_cmd.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index ff6f3a1f433..9abc9746113 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -560,7 +560,7 @@ class CommandSetEX : public Commander { class CommandPSetEX : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], 10); + auto parseResult = ParseInt(args[2], 10); if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } @@ -1109,7 +1109,7 @@ class CommandPExpire : public Commander { Status Parse(const std::vector &args) override { int64_t now; rocksdb::Env::Default()->GetCurrentTime(&now); - auto parseResult = ParseInt(args[2], 10); + auto parseResult = ParseInt(args[2], 10); if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } @@ -1522,7 +1522,7 @@ class CommandPop : public Commander { if (args.size() == 2) { return Status::OK(); } - auto parseResult = ParseInt(args[2], 10); + auto parseResult = ParseInt(args[2], 10); if (!parseResult) { return Status(Status::RedisParseErr, errValueNotInterger); } From 26da9a2df0178da9c56b3d9463b8d68752e61ae3 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 09:50:35 +0800 Subject: [PATCH 07/11] Beautification code --- src/redis_cmd.cc | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index 9abc9746113..dd6caf14c33 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -560,16 +560,15 @@ class CommandSetEX : public Commander { class CommandPSetEX : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], 10); - if (!parseResult) { + auto ttl_ms = ParseInt(args[2], 10); + if (!ttl_ms) { return Status(Status::RedisParseErr, errValueNotInterger); } - auto ttl_ms = *parseResult; - if (ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); - if (ttl_ms > 0 && ttl_ms < 1000) { + if (*ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); + if (*ttl_ms > 0 && *ttl_ms < 1000) { ttl_ = 1; } else { - ttl_ = ttl_ms / 1000; + ttl_ = *ttl_ms / 1000; } return Commander::Parse(args); } @@ -1109,15 +1108,14 @@ class CommandPExpire : public Commander { Status Parse(const std::vector &args) override { int64_t now; rocksdb::Env::Default()->GetCurrentTime(&now); - auto parseResult = ParseInt(args[2], 10); - if (!parseResult) { + auto ttl_ms = ParseInt(args[2], 10); + if (!ttl_ms) { return Status(Status::RedisParseErr, errValueNotInterger); } - auto ttl_ms = *parseResult; - if (ttl_ms > 0 && ttl_ms < 1000) { + if (*ttl_ms > 0 && *ttl_ms < 1000) { seconds_ = 1; } else { - seconds_ = ttl_ms / 1000; + seconds_ = *ttl_ms / 1000; if (seconds_ >= INT32_MAX - now) { return Status(Status::RedisParseErr, "the expire time was overflow"); } @@ -1522,15 +1520,14 @@ class CommandPop : public Commander { if (args.size() == 2) { return Status::OK(); } - auto parseResult = ParseInt(args[2], 10); - if (!parseResult) { + auto v = ParseInt(args[2], 10); + if (!v) { return Status(Status::RedisParseErr, errValueNotInterger); } - int32_t v = *parseResult; - if (v < 0) { + if (*v < 0) { return Status(Status::RedisParseErr, errValueMustBePositive); } - count_ = v; + count_ = *v; with_count_ = true; return Status::OK(); } From ebde7da4759819e7bb404fc2e867093bd21f5b88 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 12:46:41 +0800 Subject: [PATCH 08/11] fix ignore whitespace --- src/redis_hash.cc | 4 ++++ src/redis_string.cc | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/redis_hash.cc b/src/redis_hash.cc index 8514bf47ed9..91b839b272f 100644 --- a/src/redis_hash.cc +++ b/src/redis_hash.cc @@ -19,6 +19,7 @@ */ #include "redis_hash.h" +#include #include #include #include @@ -81,6 +82,9 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t if (!parseResult) { return rocksdb::Status::InvalidArgument(parseResult.Msg()); } + if (isspace(value_bytes[0])) { + return rocksdb::Status::InvalidArgument("value is not an integer"); + } old_value = *parseResult; exists = true; } diff --git a/src/redis_string.cc b/src/redis_string.cc index 0f5a0ac04cb..45626389d5d 100644 --- a/src/redis_string.cc +++ b/src/redis_string.cc @@ -280,6 +280,9 @@ rocksdb::Status String::IncrBy(const std::string &user_key, int64_t increment, i if (!parseResult) { return rocksdb::Status::InvalidArgument(parseResult.Msg()); } + if (isspace(value[0])) { + return rocksdb::Status::InvalidArgument("value is not an integer"); + } n = *parseResult; } if ((increment < 0 && n <= 0 && increment < (LLONG_MIN-n)) From 3e794917b72d63370e134e278b4ba0707b9d1e75 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Tue, 20 Sep 2022 12:58:05 +0800 Subject: [PATCH 09/11] change variable name --- src/redis_cmd.cc | 76 ++++++++++++++++++++++---------------------- src/redis_hash.cc | 8 ++--- src/redis_request.cc | 6 ++-- src/redis_string.cc | 8 ++--- src/util.cc | 16 +++++----- 5 files changed, 57 insertions(+), 57 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index dd6caf14c33..87746830588 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -489,11 +489,11 @@ class CommandSet : public Commander { } else if (opt == "px" && !ttl_ && !last_arg) { int64_t ttl_ms = 0; std::string s = args_[++i]; - auto parseResult = ParseInt(s, 10); - if (!parseResult) { + auto parse_result = ParseInt(s, 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - ttl_ms = *parseResult; + ttl_ms = *parse_result; if (ttl_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime); if (ttl_ms > 0 && ttl_ms < 1000) { ttl_ = 1; // round up the pttl to second @@ -672,11 +672,11 @@ class CommandDecr : public Commander { class CommandIncrBy : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], 10); - if (!parseResult) { + auto parse_result = ParseInt(args[2], 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - increment_ = *parseResult; + increment_ = *parse_result; return Commander::Parse(args); } @@ -720,11 +720,11 @@ class CommandIncrByFloat : public Commander { class CommandDecrBy : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], 10); - if (!parseResult) { + auto parse_result = ParseInt(args[2], 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - increment_ = *parseResult; + increment_ = *parse_result; return Commander::Parse(args); } @@ -815,11 +815,11 @@ class CommandDel : public Commander { }; Status getBitOffsetFromArgument(std::string arg, uint32_t *offset) { - auto parseResult = ParseInt(arg, 10); - if (!parseResult) { + auto parse_result = ParseInt(arg, 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - int64_t offset_arg = *parseResult; + int64_t offset_arg = *parse_result; if (offset_arg < 0 || offset_arg > UINT_MAX) { return Status(Status::RedisParseErr, "bit offset is out of range"); } @@ -883,16 +883,16 @@ class CommandBitCount : public Commander { return Status(Status::RedisParseErr, errInvalidSyntax); } if (args.size() == 4) { - auto parseArgs2Result = ParseInt(args[2], 10); - if (!parseArgs2Result) { + auto parse_start = ParseInt(args[2], 10); + if (!parse_start) { return Status(Status::RedisParseErr, errValueNotInterger); } - start_ = *parseArgs2Result; - auto parseArgs3Result = ParseInt(args[3], 10); - if (!parseArgs3Result) { + start_ = *parse_start; + auto parse_stop = ParseInt(args[3], 10); + if (!parse_stop) { return Status(Status::RedisParseErr, errValueNotInterger); } - stop_ = *parseArgs3Result; + stop_ = *parse_stop; } return Commander::Parse(args); } @@ -1170,14 +1170,14 @@ class CommandExpireAt : public Commander { class CommandPExpireAt : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[2], 10); - if (!parseResult) { + auto parse_result = ParseInt(args[2], 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - if (*parseResult/1000 >= INT32_MAX) { + if (*parse_result/1000 >= INT32_MAX) { return Status(Status::RedisParseErr, "the expire time was overflow"); } - timestamp_ = static_cast(*parseResult/1000); + timestamp_ = static_cast(*parse_result/1000); return Commander::Parse(args); } Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -1304,11 +1304,11 @@ class CommandHLen : public Commander { class CommandHIncrBy : public Commander { public: Status Parse(const std::vector &args) override { - auto parseResult = ParseInt(args[3], 10); - if (!parseResult) { + auto parse_result = ParseInt(args[3], 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - increment_ = *parseResult; + increment_ = *parse_result; return Commander::Parse(args); } Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -3964,11 +3964,11 @@ class CommandClient : public Commander { if (!strcasecmp(args[i].c_str(), "addr") && moreargs) { addr_ = args[i+1]; } else if (!strcasecmp(args[i].c_str(), "id") && moreargs) { - auto parseResult = ParseInt(args[i+1], 10); - if (!parseResult) { + auto parse_result = ParseInt(args[i+1], 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - id_ = *parseResult; + id_ = *parse_result; } else if (!strcasecmp(args[i].c_str(), "skipme") && moreargs) { if (!strcasecmp(args[i+1].c_str(), "yes")) { skipme_ = true; @@ -4160,12 +4160,12 @@ class CommandHello final : public Commander { size_t next_arg = 1; if (args_.size() >= 2) { int64_t protocol; - auto parseResult = ParseInt(args_[next_arg], 10); + auto parse_result = ParseInt(args_[next_arg], 10); ++next_arg; - if (!parseResult) { + if (!parse_result) { return Status(Status::NotOK, "Protocol version is not an integer or out of range"); } - protocol = *parseResult; + protocol = *parse_result; // In redis, it will check protocol < 2 or protocol > 3, // kvrocks only supports REPL2 by now, but for supporting some @@ -5366,11 +5366,11 @@ class CommandXRead : public Commander { return Status(Status::RedisParseErr, errInvalidSyntax); } with_count_ = true; - auto parseResult = ParseInt(args[i+1], 10); - if (!parseResult) { + auto parse_result = ParseInt(args[i+1], 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - count_ = *parseResult; + count_ = *parse_result; i += 2; continue; } @@ -5381,14 +5381,14 @@ class CommandXRead : public Commander { } block_ = true; - auto parseResult = ParseInt(args[i+1], 10); - if (!parseResult) { + auto parse_result = ParseInt(args[i+1], 10); + if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } - if (*parseResult < 0) { + if (*parse_result < 0) { return Status(Status::RedisParseErr, errTimeoutIsNegative); } - block_timeout_ = *parseResult; + block_timeout_ = *parse_result; i += 2; continue; } diff --git a/src/redis_hash.cc b/src/redis_hash.cc index 91b839b272f..243645994ff 100644 --- a/src/redis_hash.cc +++ b/src/redis_hash.cc @@ -78,14 +78,14 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t s = db_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes); if (!s.ok() && !s.IsNotFound()) return s; if (s.ok()) { - auto parseResult = ParseInt(value_bytes, 10); - if (!parseResult) { - return rocksdb::Status::InvalidArgument(parseResult.Msg()); + auto parse_result = ParseInt(value_bytes, 10); + if (!parse_result) { + return rocksdb::Status::InvalidArgument(parse_result.Msg()); } if (isspace(value_bytes[0])) { return rocksdb::Status::InvalidArgument("value is not an integer"); } - old_value = *parseResult; + old_value = *parse_result; exists = true; } } diff --git a/src/redis_request.cc b/src/redis_request.cc index 9e094ab134b..47ff5095c75 100644 --- a/src/redis_request.cc +++ b/src/redis_request.cc @@ -67,11 +67,11 @@ Status Request::Tokenize(evbuffer *input) { pipeline_size++; svr_->stats_.IncrInbondBytes(line.length); if (line[0] == '*') { - auto parseResult = ParseInt(std::string(line.get() + 1, line.length - 1), 10); - if (!parseResult) { + auto parse_result = ParseInt(std::string(line.get() + 1, line.length - 1), 10); + if (!parse_result) { return Status(Status::NotOK, "Protocol error: invalid multibulk length"); } - multi_bulk_len_ = *parseResult; + multi_bulk_len_ = *parse_result; if (isOnlyLF || multi_bulk_len_ > (int64_t)PROTO_MULTI_MAX_SIZE) { return Status(Status::NotOK, "Protocol error: invalid multibulk length"); } diff --git a/src/redis_string.cc b/src/redis_string.cc index 45626389d5d..02ef3d16506 100644 --- a/src/redis_string.cc +++ b/src/redis_string.cc @@ -276,14 +276,14 @@ rocksdb::Status String::IncrBy(const std::string &user_key, int64_t increment, i value = raw_value.substr(STRING_HDR_SIZE, raw_value.size()-STRING_HDR_SIZE); int64_t n = 0; if (!value.empty()) { - auto parseResult = ParseInt(value, 10); - if (!parseResult) { - return rocksdb::Status::InvalidArgument(parseResult.Msg()); + auto parse_result = ParseInt(value, 10); + if (!parse_result) { + return rocksdb::Status::InvalidArgument(parse_result.Msg()); } if (isspace(value[0])) { return rocksdb::Status::InvalidArgument("value is not an integer"); } - n = *parseResult; + n = *parse_result; } if ((increment < 0 && n <= 0 && increment < (LLONG_MIN-n)) || (increment > 0 && n >= 0 && increment > (LLONG_MAX-n))) { diff --git a/src/util.cc b/src/util.cc index a44c9b29bc3..7c80ab9826c 100644 --- a/src/util.cc +++ b/src/util.cc @@ -342,20 +342,20 @@ int GetLocalPort(int fd) { } Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - auto parseResult = ParseInt(str, NumericRange{min, max}, 10); - if (!parseResult) { - return Status(Status::NotOK, parseResult.Msg()); + auto parse_result = ParseInt(str, NumericRange{min, max}, 10); + if (!parse_result) { + return parse_result.ToStatus(); } - *n = *parseResult; + *n = *parse_result; return Status::OK(); } Status OctalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) { - auto parseResult = ParseInt(str, NumericRange{min, max}, 8); - if (!parseResult) { - return Status(Status::NotOK, parseResult.Msg()); + auto parse_result = ParseInt(str, NumericRange{min, max}, 8); + if (!parse_result) { + return parse_result.ToStatus(); } - *n = *parseResult; + *n = *parse_result; return Status::OK(); } From b5f11ad075f938b951e147a26e318b6ef9c2812e Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Wed, 21 Sep 2022 20:20:42 +0800 Subject: [PATCH 10/11] fix --- src/redis_cmd.cc | 25 +++++++++++-------------- src/redis_string.cc | 2 +- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index 3f35b4a2c5c..9e6d1771f8d 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -817,15 +817,12 @@ class CommandDel : public Commander { }; Status getBitOffsetFromArgument(std::string arg, uint32_t *offset) { - auto parse_result = ParseInt(arg, 10); + auto parse_result = ParseInt(arg, 10); if (!parse_result) { - return Status(Status::RedisParseErr, errValueNotInterger); + return parse_result.ToStatus(); } - int64_t offset_arg = *parse_result; - if (offset_arg < 0 || offset_arg > UINT_MAX) { - return Status(Status::RedisParseErr, "bit offset is out of range"); - } - *offset = static_cast(offset_arg); + + *offset = *parse_result; return Status::OK(); } @@ -916,19 +913,19 @@ class CommandBitPos: public Commander { public: Status Parse(const std::vector &args) override { if (args.size() >= 4) { - auto parseArgs3Result = ParseInt(args[3], 10); - if (!parseArgs3Result) { + auto parse_start = ParseInt(args[3], 10); + if (!parse_start) { return Status(Status::RedisParseErr, errValueNotInterger); } - start_ = *parseArgs3Result; + start_ = *parse_start; } if (args.size() >= 5) { - auto parseArgs4Result = ParseInt(args[4], 10); - if (!parseArgs4Result) { + auto parse_stop = ParseInt(args[4], 10); + if (!parse_stop) { return Status(Status::RedisParseErr, errValueNotInterger); } stop_given_ = true; - stop_ = *parseArgs4Result; + stop_ = *parse_stop; } if (args[2] == "0") { bit_ = false; @@ -4007,7 +4004,7 @@ class CommandClient : public Commander { if (!strcasecmp(args[i].c_str(), "addr") && moreargs) { addr_ = args[i+1]; } else if (!strcasecmp(args[i].c_str(), "id") && moreargs) { - auto parse_result = ParseInt(args[i+1], 10); + auto parse_result = ParseInt(args[i+1], 10); if (!parse_result) { return Status(Status::RedisParseErr, errValueNotInterger); } diff --git a/src/redis_string.cc b/src/redis_string.cc index ff9c8eae3ab..5c882687be9 100644 --- a/src/redis_string.cc +++ b/src/redis_string.cc @@ -278,7 +278,7 @@ rocksdb::Status String::IncrBy(const std::string &user_key, int64_t increment, i if (!value.empty()) { auto parse_result = ParseInt(value, 10); if (!parse_result) { - return rocksdb::Status::InvalidArgument(parse_result.Msg()); + return rocksdb::Status::InvalidArgument("value is not an integer or out of range"); } if (isspace(value[0])) { return rocksdb::Status::InvalidArgument("value is not an integer"); From 8b107bdb032c92bee4aa244e489ad4cb626db1b7 Mon Sep 17 00:00:00 2001 From: tanruixiang <819464715@qq.com> Date: Wed, 21 Sep 2022 21:02:54 +0800 Subject: [PATCH 11/11] update error message --- tests/tcl/tests/unit/type/hash.tcl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tcl/tests/unit/type/hash.tcl b/tests/tcl/tests/unit/type/hash.tcl index e34ece147b7..0352eaeb3ec 100644 --- a/tests/tcl/tests/unit/type/hash.tcl +++ b/tests/tcl/tests/unit/type/hash.tcl @@ -335,8 +335,8 @@ start_server {tags {"hash"}} { catch {r hincrby smallhash str 1} smallerr catch {r hincrby smallhash str 1} bigerr set rv {} - lappend rv [string match "ERR*not an integer*" $smallerr] - lappend rv [string match "ERR*not an integer*" $bigerr] + lappend rv [string match "ERR*non-integer*" $smallerr] + lappend rv [string match "ERR*non-integer*" $bigerr] } {1 1} test {HINCRBY can detect overflows} {