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/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 360476df Enhance error handling and types in BITFIELD (#2041)
360476df is described below
commit 360476dffc52a60b88191f29115269c0655e8359
Author: mwish <[email protected]>
AuthorDate: Wed Jan 24 22:10:40 2024 +0800
Enhance error handling and types in BITFIELD (#2041)
Co-authored-by: hulk <[email protected]>
Co-authored-by: Twice <[email protected]>
---
src/common/bitfield_util.h | 12 ++++++------
src/types/redis_bitmap_string.cc | 18 +++++++++++++++---
tests/cppunit/bitfield_util.cc | 2 +-
3 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/src/common/bitfield_util.h b/src/common/bitfield_util.h
index dbb44b1d..63d27980 100644
--- a/src/common/bitfield_util.h
+++ b/src/common/bitfield_util.h
@@ -205,7 +205,7 @@ class ArrayBitfieldBitmap {
void Reset() { memset(buf_, 0, sizeof(buf_)); }
- Status Set(uint32_t byte_offset, uint32_t bytes, const uint8_t *src) {
+ [[nodiscard]] Status Set(uint32_t byte_offset, uint32_t bytes, const uint8_t
*src) {
Status bound_status(checkLegalBound(byte_offset, bytes));
if (!bound_status) {
return bound_status;
@@ -215,7 +215,7 @@ class ArrayBitfieldBitmap {
return Status::OK();
}
- Status Get(uint32_t byte_offset, uint32_t bytes, uint8_t *dst) const {
+ [[nodiscard]] Status Get(uint32_t byte_offset, uint32_t bytes, uint8_t *dst)
const {
Status bound_status(checkLegalBound(byte_offset, bytes));
if (!bound_status) {
return bound_status;
@@ -226,7 +226,7 @@ class ArrayBitfieldBitmap {
}
StatusOr<uint64_t> GetUnsignedBitfield(uint64_t bit_offset, uint64_t bits)
const {
- Status
bits_status(BitfieldEncoding::CheckSupportedBitLengths(BitfieldEncoding::Type::kUnsigned,
bits));
+ Status bits_status =
BitfieldEncoding::CheckSupportedBitLengths(BitfieldEncoding::Type::kUnsigned,
bits);
if (!bits_status) {
return bits_status;
}
@@ -234,7 +234,7 @@ class ArrayBitfieldBitmap {
}
StatusOr<int64_t> GetSignedBitfield(uint64_t bit_offset, uint64_t bits)
const {
- Status
bits_status(BitfieldEncoding::CheckSupportedBitLengths(BitfieldEncoding::Type::kSigned,
bits));
+ Status bits_status =
BitfieldEncoding::CheckSupportedBitLengths(BitfieldEncoding::Type::kSigned,
bits);
if (!bits_status) {
return bits_status;
}
@@ -257,10 +257,10 @@ class ArrayBitfieldBitmap {
return value;
}
- Status SetBitfield(uint32_t bit_offset, uint32_t bits, uint64_t value) {
+ [[nodiscard]] Status SetBitfield(uint32_t bit_offset, uint32_t bits,
uint64_t value) {
uint32_t first_byte = bit_offset / 8;
uint32_t last_byte = (bit_offset + bits - 1) / 8 + 1;
- Status bound_status(checkLegalBound(first_byte, last_byte - first_byte));
+ Status bound_status = checkLegalBound(first_byte, last_byte - first_byte);
if (!bound_status) {
return bound_status;
}
diff --git a/src/types/redis_bitmap_string.cc b/src/types/redis_bitmap_string.cc
index d9d77114..9b179630 100644
--- a/src/types/redis_bitmap_string.cc
+++ b/src/types/redis_bitmap_string.cc
@@ -222,7 +222,10 @@ rocksdb::Status BitmapString::Bitfield(const Slice
&ns_key, std::string *raw_val
ArrayBitfieldBitmap bitfield(first_byte);
auto str = reinterpret_cast<const uint8_t *>(string_value.data() +
first_byte);
- auto s = bitfield.Set(first_byte, last_byte - first_byte, str);
+ auto s = bitfield.Set(/*byte_offset=*/first_byte, /*bytes=*/last_byte -
first_byte, /*src=*/str);
+ if (!s.IsOK()) {
+ return rocksdb::Status::IOError(s.Msg());
+ }
uint64_t unsigned_old_value = 0;
if (op.encoding.IsSigned()) {
@@ -232,13 +235,19 @@ rocksdb::Status BitmapString::Bitfield(const Slice
&ns_key, std::string *raw_val
}
uint64_t unsigned_new_value = 0;
- auto &ret = rets->emplace_back();
- if (BitfieldOp(op, unsigned_old_value, &unsigned_new_value).GetValue()) {
+ std::optional<BitfieldValue> &ret = rets->emplace_back();
+ StatusOr<bool> bitfield_op = BitfieldOp(op, unsigned_old_value,
&unsigned_new_value);
+ if (!bitfield_op.IsOK()) {
+ return rocksdb::Status::InvalidArgument(bitfield_op.Msg());
+ }
+ if (bitfield_op.GetValue()) {
if (op.type != BitfieldOperation::Type::kGet) {
// never failed.
s = bitfield.SetBitfield(op.offset, op.encoding.Bits(),
unsigned_new_value);
+ CHECK(s.IsOK());
auto dst = reinterpret_cast<uint8_t *>(string_value.data()) +
first_byte;
s = bitfield.Get(first_byte, last_byte - first_byte, dst);
+ CHECK(s.IsOK());
}
if (op.type == BitfieldOperation::Type::kSet) {
@@ -276,6 +285,9 @@ rocksdb::Status BitmapString::BitfieldReadOnly(const Slice
&ns_key, const std::s
ArrayBitfieldBitmap bitfield(first_byte);
auto s = bitfield.Set(first_byte, last_byte - first_byte,
reinterpret_cast<const uint8_t
*>(string_value.data() + first_byte));
+ if (!s.IsOK()) {
+ return rocksdb::Status::IOError(s.Msg());
+ }
if (op.encoding.IsSigned()) {
int64_t value = bitfield.GetSignedBitfield(op.offset,
op.encoding.Bits()).GetValue();
diff --git a/tests/cppunit/bitfield_util.cc b/tests/cppunit/bitfield_util.cc
index 71976df4..c6b9d36d 100644
--- a/tests/cppunit/bitfield_util.cc
+++ b/tests/cppunit/bitfield_util.cc
@@ -35,7 +35,7 @@ TEST(BitfieldUtil, Get) {
ArrayBitfieldBitmap bitfield(0);
auto s = bitfield.Set(0, big_endian_bitmap.size(), big_endian_bitmap.data());
- for (int bits = 16; bits < 64; bits *= 2) {
+ for (uint8_t bits = 16; bits < 64; bits *= 2) {
for (uint64_t offset = 0; bits + offset <= big_endian_bitmap.size() * 8;
offset += bits) {
uint64_t value = bitfield.GetUnsignedBitfield(offset, bits).GetValue();
if (IsBigEndian()) {
