You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by tw...@apache.org on 2022/09/25 08:04:33 UTC
[incubator-kvrocks] branch unstable updated: Replace `std::stol` with ParseInt (#897)
This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 6d8e8f5 Replace `std::stol` with ParseInt (#897)
6d8e8f5 is described below
commit 6d8e8f56269ac5e77e3dba88feeae2f1fb6e54db
Author: Ruixiang Tan <81...@qq.com>
AuthorDate: Sun Sep 25 16:04:27 2022 +0800
Replace `std::stol` with ParseInt (#897)
---
src/redis_cmd.cc | 177 ++++++++++++++++++-------------------
src/redis_hash.cc | 13 +--
src/redis_request.cc | 7 +-
src/redis_string.cc | 10 +--
src/util.cc | 23 ++---
tests/cppunit/t_hash_test.cc | 8 +-
tests/tcl/tests/unit/type/hash.tcl | 4 +-
7 files changed, 120 insertions(+), 122 deletions(-)
diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc
index 553c77f..9e6d177 100644
--- a/src/redis_cmd.cc
+++ b/src/redis_cmd.cc
@@ -490,16 +490,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 parse_result = ParseInt<int64_t>(s, 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ 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
@@ -566,17 +562,16 @@ class CommandSetEX : public Commander {
class CommandPSetEX : public Commander {
public:
Status Parse(const std::vector<std::string> &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 ttl_ms = ParseInt<int64_t>(args[2], 10);
+ if (!ttl_ms) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ 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);
}
@@ -679,11 +674,11 @@ class CommandDecr : public Commander {
class CommandIncrBy : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
- try {
- increment_ = std::stoll(args[2]);
- } catch (std::exception &e) {
+ auto parse_result = ParseInt<int64_t>(args[2], 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ increment_ = *parse_result;
return Commander::Parse(args);
}
@@ -727,11 +722,11 @@ class CommandIncrByFloat : public Commander {
class CommandDecrBy : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
- try {
- increment_ = std::stoll(args[2]);
- } catch (std::exception &e) {
+ auto parse_result = ParseInt<int64_t>(args[2], 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ increment_ = *parse_result;
return Commander::Parse(args);
}
@@ -822,16 +817,12 @@ 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) {
- return Status(Status::RedisParseErr, errValueNotInterger);
- }
- if (offset_arg < 0 || offset_arg > UINT_MAX) {
- return Status(Status::RedisParseErr, "bit offset is out of range");
+ auto parse_result = ParseInt<uint32_t>(arg, 10);
+ if (!parse_result) {
+ return parse_result.ToStatus();
}
- *offset = static_cast<uint32_t>(offset_arg);
+
+ *offset = *parse_result;
return Status::OK();
}
@@ -891,12 +882,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 parse_start = ParseInt<int64_t>(args[2], 10);
+ if (!parse_start) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ start_ = *parse_start;
+ auto parse_stop = ParseInt<int64_t>(args[3], 10);
+ if (!parse_stop) {
+ return Status(Status::RedisParseErr, errValueNotInterger);
+ }
+ stop_ = *parse_stop;
}
return Commander::Parse(args);
}
@@ -917,14 +912,20 @@ class CommandBitCount : public Commander {
class CommandBitPos: public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
- try {
- if (args.size() >= 4) start_ = std::stol(args[3]);
- if (args.size() >= 5) {
- stop_given_ = true;
- stop_ = std::stol(args[4]);
+ if (args.size() >= 4) {
+ auto parse_start = ParseInt<int64_t>(args[3], 10);
+ if (!parse_start) {
+ return Status(Status::RedisParseErr, errValueNotInterger);
}
- } catch (std::exception &e) {
- return Status(Status::RedisParseErr, errValueNotInterger);
+ start_ = *parse_start;
+ }
+ if (args.size() >= 5) {
+ auto parse_stop = ParseInt<int64_t>(args[4], 10);
+ if (!parse_stop) {
+ return Status(Status::RedisParseErr, errValueNotInterger);
+ }
+ stop_given_ = true;
+ stop_ = *parse_stop;
}
if (args[2] == "0") {
bit_ = false;
@@ -1106,20 +1107,19 @@ class CommandPExpire : public Commander {
Status Parse(const std::vector<std::string> &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 ttl_ms = ParseInt<int64_t>(args[2], 10);
+ if (!ttl_ms) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ 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);
}
@@ -1169,14 +1169,14 @@ class CommandExpireAt : public Commander {
class CommandPExpireAt : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
- try {
- timestamp_ = static_cast<int>(std::stol(args[2])/1000);
- if (timestamp_ >= INT32_MAX) {
- return Status(Status::RedisParseErr, "the expire time was overflow");
- }
- } catch (std::exception &e) {
+ auto parse_result = ParseInt<int64_t>(args[2], 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ if (*parse_result/1000 >= INT32_MAX) {
+ return Status(Status::RedisParseErr, "the expire time was overflow");
+ }
+ timestamp_ = static_cast<int>(*parse_result/1000);
return Commander::Parse(args);
}
Status Execute(Server *svr, Connection *conn, std::string *output) override {
@@ -1303,11 +1303,11 @@ class CommandHLen : public Commander {
class CommandHIncrBy : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
- try {
- increment_ = std::stoll(args[3]);
- } catch (std::exception &e) {
+ auto parse_result = ParseInt<int64_t>(args[3], 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ increment_ = *parse_result;
return Commander::Parse(args);
}
Status Execute(Server *svr, Connection *conn, std::string *output) override {
@@ -1557,16 +1557,15 @@ 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 v = ParseInt<int32_t>(args[2], 10);
+ if (!v) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ if (*v < 0) {
+ return Status(Status::RedisParseErr, errValueMustBePositive);
+ }
+ count_ = *v;
+ with_count_ = true;
return Status::OK();
}
@@ -4005,11 +4004,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 parse_result = ParseInt<uint64_t>(args[i+1], 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ id_ = *parse_result;
} else if (!strcasecmp(args[i].c_str(), "skipme") && moreargs) {
if (!strcasecmp(args[i+1].c_str(), "yes")) {
skipme_ = true;
@@ -4201,12 +4200,12 @@ class CommandHello final : public Commander {
size_t next_arg = 1;
if (args_.size() >= 2) {
int64_t protocol;
- auto parseResult = ParseInt<int64_t>(args_[next_arg], /* base= */ 10);
+ auto parse_result = ParseInt<int64_t>(args_[next_arg], 10);
++next_arg;
- if (!parseResult.IsOK()) {
+ if (!parse_result) {
return Status(Status::NotOK, "Protocol version is not an integer or out of range");
}
- protocol = parseResult.GetValue();
+ protocol = *parse_result;
// In redis, it will check protocol < 2 or protocol > 3,
// kvrocks only supports REPL2 by now, but for supporting some
@@ -5428,13 +5427,12 @@ class CommandXRead : public Commander {
if (i+1 >= args.size()) {
return Status(Status::RedisParseErr, errInvalidSyntax);
}
-
- try {
- with_count_ = true;
- count_ = static_cast<uint64_t>(std::stoll(args[i+1]));
- } catch (const std::exception &) {
+ with_count_ = true;
+ auto parse_result = ParseInt<uint64_t>(args[i+1], 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ count_ = *parse_result;
i += 2;
continue;
}
@@ -5445,15 +5443,14 @@ 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 parse_result = ParseInt<int64_t>(args[i+1], 10);
+ if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInterger);
}
+ if (*parse_result < 0) {
+ return Status(Status::RedisParseErr, errTimeoutIsNegative);
+ }
+ block_timeout_ = *parse_result;
i += 2;
continue;
}
diff --git a/src/redis_hash.cc b/src/redis_hash.cc
index 0fbf407..7faadbc 100644
--- a/src/redis_hash.cc
+++ b/src/redis_hash.cc
@@ -19,6 +19,7 @@
*/
#include "redis_hash.h"
+#include <cctype>
#include <utility>
#include <algorithm>
#include <limits>
@@ -26,6 +27,7 @@
#include <iostream>
#include <rocksdb/status.h>
#include "db_util.h"
+#include "parse_util.h"
namespace Redis {
rocksdb::Status Hash::GetMetadata(const Slice &ns_key, HashMetadata *metadata) {
@@ -74,18 +76,17 @@ 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());
+ auto parse_result = ParseInt<int64_t>(value_bytes, 10);
+ if (!parse_result) {
+ return rocksdb::Status::InvalidArgument(parse_result.Msg());
}
- if (isspace(value_bytes[0]) || idx != value_bytes.size()) {
+ if (isspace(value_bytes[0])) {
return rocksdb::Status::InvalidArgument("value is not an integer");
}
+ old_value = *parse_result;
exists = true;
}
}
diff --git a/src/redis_request.cc b/src/redis_request.cc
index 88a18c4..47ff509 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 parse_result = ParseInt<int64_t>(std::string(line.get() + 1, line.length - 1), 10);
+ if (!parse_result) {
return Status(Status::NotOK, "Protocol error: invalid multibulk length");
}
+ 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 67e5e0d..5c88268 100644
--- a/src/redis_string.cc
+++ b/src/redis_string.cc
@@ -23,6 +23,7 @@
#include <string>
#include <limits>
#include <cmath>
+#include "parse_util.h"
namespace Redis {
@@ -274,16 +275,15 @@ 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) {
+ auto parse_result = ParseInt<int64_t>(value, 10);
+ if (!parse_result) {
return rocksdb::Status::InvalidArgument("value is not an integer or out of range");
}
- if (isspace(value[0]) || idx != value.size()) {
+ if (isspace(value[0])) {
return rocksdb::Status::InvalidArgument("value is not an integer");
}
+ 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 31944f0..7c80ab9 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,20 @@ int GetLocalPort(int fd) {
}
Status DecimalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) {
- try {
- *n = static_cast<int64_t>(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) {
- return Status(Status::NotOK, "value is not an integer or out of range");
+ auto parse_result = ParseInt<int64_t>(str, NumericRange<int64_t>{min, max}, 10);
+ if (!parse_result) {
+ return parse_result.ToStatus();
}
+ *n = *parse_result;
return Status::OK();
}
Status OctalStringToNum(const std::string &str, int64_t *n, int64_t min, int64_t max) {
- try {
- *n = static_cast<int64_t>(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) {
- return Status(Status::NotOK, "value is not an integer or out of range");
+ auto parse_result = ParseInt<int64_t>(str, NumericRange<int64_t>{min, max}, 8);
+ if (!parse_result) {
+ return parse_result.ToStatus();
}
+ *n = *parse_result;
return Status::OK();
}
diff --git a/tests/cppunit/t_hash_test.cc b/tests/cppunit/t_hash_test.cc
index 27f612b..9ff9b9f 100644
--- a/tests/cppunit/t_hash_test.cc
+++ b/tests/cppunit/t_hash_test.cc
@@ -26,6 +26,7 @@
#include "test_base.h"
#include "redis_hash.h"
+#include "parse_util.h"
class RedisHashTest : public TestBase {
protected:
explicit RedisHashTest() : TestBase() {
@@ -117,8 +118,11 @@ TEST_F(RedisHashTest, HIncr) {
}
std::string bytes;
hash->Get(key_, field, &bytes);
- value = std::stoll(bytes);
- EXPECT_EQ(32, value);
+ auto parseResult = ParseInt<int64_t>(bytes, 10);
+ if (!parseResult) {
+ FAIL();
+ }
+ EXPECT_EQ(32, *parseResult);
hash->Del(key_);
}
diff --git a/tests/tcl/tests/unit/type/hash.tcl b/tests/tcl/tests/unit/type/hash.tcl
index e34ece1..0352eae 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} {