Skip to content

Commit d658a30

Browse files
678098678098
authored
Refactor[BMQ]: simplify bmqu::BlobUtil (#1021)
Signed-off-by: Evgeny Malygin <[email protected]>
1 parent 157c079 commit d658a30

File tree

4 files changed

+34
-45
lines changed

4 files changed

+34
-45
lines changed

‎src/groups/bmq/bmqp/bmqp_messageproperties.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,7 @@ MessageProperties::setProperty(const bsl::string& name, const TYPE& value)
696696

697697
p.d_length = newPropValueLen;
698698
p.d_value = value;
699-
// This cannot have bsl::string_view' type.
699+
// This cannot have `bsl::string_view` type.
700700
p.d_type = static_cast<bmqt::PropertyType::Enum>(p.d_value.typeIndex());
701701
p.d_isValid = true;
702702
d_isDirty = true;

‎src/groups/bmq/bmqu/bmqu_blob.cpp‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2022-2023 Bloomberg Finance L.P.
1+
// Copyright 2014-2025 Bloomberg Finance L.P.
22
// SPDX-License-Identifier: Apache-2.0
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
@@ -86,7 +86,7 @@ bool BlobUtil::isValidPos(const bdlbb::Blob& blob, const BlobPosition& pos)
8686
return pos.byte() == 0; // RETURN
8787
}
8888

89-
return pos.byte() >= 0 && pos.byte() < bufferSize(blob, pos.buffer());
89+
return 0 <= pos.byte() && pos.byte() < bufferSize(blob, pos.buffer());
9090
}
9191

9292
int BlobUtil::findOffset(BlobPosition* pos,
@@ -123,9 +123,7 @@ int BlobUtil::findOffset(BlobPosition* pos,
123123
}
124124
}
125125

126-
// Looking past the end of the blob (return -3 since the 'safe' version
127-
// already uses -1 for invalid 'start' and -2 for invalid 'offset').
128-
return -3;
126+
return -2;
129127
}
130128

131129
bool BlobUtil::isValidSection(const bdlbb::Blob& blob,
@@ -494,12 +492,6 @@ int BlobUtil::readUpToNBytes(char* buf,
494492
return -1; // RETURN
495493
}
496494

497-
if (BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY(
498-
start.buffer() == blob.numDataBuffers() && length > 0)) {
499-
BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
500-
return -2; // RETURN
501-
}
502-
503495
if (BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY(length == 0)) {
504496
BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
505497
return 0; // RETURN

‎src/groups/bmq/bmqu/bmqu_blob.h‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2022-2023 Bloomberg Finance L.P.
1+
// Copyright 2014-2025 Bloomberg Finance L.P.
22
// SPDX-License-Identifier: Apache-2.0
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
@@ -215,6 +215,12 @@ struct BlobUtil {
215215
static bool isValidSection(const bdlbb::Blob& blob,
216216
const BlobSection& section);
217217

218+
/// @brief Check if the data segment defined by `start` and `end`
219+
/// positions is continous (placed in the same blob buffer).
220+
/// @param start the start position in a blob
221+
/// @param end the end position in a blob
222+
/// @return true if the segment is continuous, false otherwise.
223+
/// NOTE: this function doesn't check if blob positions are valid.
218224
static bool isDataContinuous(const bmqu::BlobPosition& start,
219225
const bmqu::BlobPosition& end);
220226

@@ -578,14 +584,6 @@ inline int BlobUtil::findOffsetSafe(BlobPosition* pos,
578584
return -1; // RETURN
579585
}
580586

581-
// Check if 'offset' has a proper value if 'start' position points to
582-
// the buffer past the last one of the blob.
583-
if (BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY(
584-
start.buffer() == blob.numDataBuffers() && offset > 0)) {
585-
BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
586-
return -2; // RETURN
587-
}
588-
589587
return findOffset(pos, blob, start, offset);
590588
}
591589

‎src/groups/bmq/bmqu/bmqu_blob.t.cpp‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,8 @@ static void test7_findOffset()
473473
{L_, 8, 0, bmqu::BlobPosition(3, 2)},
474474
{L_, 9, 0, bmqu::BlobPosition(4, 0)},
475475
// Invalid offsets
476-
{L_, 10, -3, bmqu::BlobPosition(0, 0)},
477-
{L_, 11, -3, bmqu::BlobPosition(0, 0)},
476+
{L_, 10, -2, bmqu::BlobPosition(0, 0)},
477+
{L_, 11, -2, bmqu::BlobPosition(0, 0)},
478478
};
479479

480480
const size_t k_NUM_DATA = sizeof(k_DATA) / sizeof(*k_DATA);
@@ -529,8 +529,8 @@ static void test7_findOffset()
529529
{L_, bmqu::BlobPosition(0, 1), 6, 0, bmqu::BlobPosition(3, 1)},
530530
{L_, bmqu::BlobPosition(0, 1), 7, 0, bmqu::BlobPosition(3, 2)},
531531
{L_, bmqu::BlobPosition(0, 1), 8, 0, bmqu::BlobPosition(4, 0)},
532-
{L_, bmqu::BlobPosition(0, 1), 9, -3, bmqu::BlobPosition(0, 0)},
533-
{L_, bmqu::BlobPosition(0, 1), 10, -3, bmqu::BlobPosition(0, 0)},
532+
{L_, bmqu::BlobPosition(0, 1), 9, -2, bmqu::BlobPosition(0, 0)},
533+
{L_, bmqu::BlobPosition(0, 1), 10, -2, bmqu::BlobPosition(0, 0)},
534534
// Start at (1,0)
535535
{L_, bmqu::BlobPosition(1, 0), 0, 0, bmqu::BlobPosition(1, 0)},
536536
{L_, bmqu::BlobPosition(1, 0), 1, 0, bmqu::BlobPosition(1, 1)},
@@ -540,22 +540,22 @@ static void test7_findOffset()
540540
{L_, bmqu::BlobPosition(1, 0), 5, 0, bmqu::BlobPosition(3, 1)},
541541
{L_, bmqu::BlobPosition(1, 0), 6, 0, bmqu::BlobPosition(3, 2)},
542542
{L_, bmqu::BlobPosition(1, 0), 7, 0, bmqu::BlobPosition(4, 0)},
543-
{L_, bmqu::BlobPosition(1, 0), 8, -3, bmqu::BlobPosition(0, 0)},
544-
{L_, bmqu::BlobPosition(1, 0), 9, -3, bmqu::BlobPosition(0, 0)},
543+
{L_, bmqu::BlobPosition(1, 0), 8, -2, bmqu::BlobPosition(0, 0)},
544+
{L_, bmqu::BlobPosition(1, 0), 9, -2, bmqu::BlobPosition(0, 0)},
545545
// Start at (1, 2)
546546
{L_, bmqu::BlobPosition(1, 2), 0, 0, bmqu::BlobPosition(1, 2)},
547547
{L_, bmqu::BlobPosition(1, 2), 1, 0, bmqu::BlobPosition(2, 0)},
548548
{L_, bmqu::BlobPosition(1, 2), 2, 0, bmqu::BlobPosition(3, 0)},
549549
{L_, bmqu::BlobPosition(1, 2), 3, 0, bmqu::BlobPosition(3, 1)},
550550
{L_, bmqu::BlobPosition(1, 2), 4, 0, bmqu::BlobPosition(3, 2)},
551551
{L_, bmqu::BlobPosition(1, 2), 5, 0, bmqu::BlobPosition(4, 0)},
552-
{L_, bmqu::BlobPosition(1, 2), 6, -3, bmqu::BlobPosition(0, 0)},
553-
{L_, bmqu::BlobPosition(1, 2), 7, -3, bmqu::BlobPosition(0, 0)},
552+
{L_, bmqu::BlobPosition(1, 2), 6, -2, bmqu::BlobPosition(0, 0)},
553+
{L_, bmqu::BlobPosition(1, 2), 7, -2, bmqu::BlobPosition(0, 0)},
554554
// Start at (3, 2)
555555
{L_, bmqu::BlobPosition(3, 2), 0, 0, bmqu::BlobPosition(3, 2)},
556556
{L_, bmqu::BlobPosition(3, 2), 1, 0, bmqu::BlobPosition(4, 0)},
557-
{L_, bmqu::BlobPosition(3, 2), 2, -3, bmqu::BlobPosition(0, 0)},
558-
{L_, bmqu::BlobPosition(3, 2), 3, -3, bmqu::BlobPosition(0, 0)},
557+
{L_, bmqu::BlobPosition(3, 2), 2, -2, bmqu::BlobPosition(0, 0)},
558+
{L_, bmqu::BlobPosition(3, 2), 3, -2, bmqu::BlobPosition(0, 0)},
559559
// Start at (4, 0) (i.e. one past the end)
560560
{L_, bmqu::BlobPosition(4, 0), 0, 0, bmqu::BlobPosition(4, 0)},
561561
{L_, bmqu::BlobPosition(4, 0), 1, -2, bmqu::BlobPosition(0, 0)},
@@ -715,19 +715,19 @@ static void test9_writeBytes()
715715
{L_, "ab|cde", BlobPosition(0, 0), "123", "123de", 0},
716716
{L_, "ab|cde", BlobPosition(0, 0), "1234", "1234e", 0},
717717
{L_, "ab|cde", BlobPosition(0, 0), "12345", "12345", 0},
718-
{L_, "ab|cde", BlobPosition(0, 0), "123456", "abcde", -31},
718+
{L_, "ab|cde", BlobPosition(0, 0), "123456", "abcde", -21},
719719
{L_, "ab|cde", BlobPosition(0, 1), "1", "a1cde", 0},
720720
{L_, "ab|cde", BlobPosition(0, 1), "12", "a12de", 0},
721721
{L_, "ab|cde", BlobPosition(0, 1), "123", "a123e", 0},
722722
{L_, "ab|cde", BlobPosition(0, 1), "1234", "a1234", 0},
723-
{L_, "ab|cde", BlobPosition(0, 1), "12345", "abcde", -31},
723+
{L_, "ab|cde", BlobPosition(0, 1), "12345", "abcde", -21},
724724
{L_, "ab|cde", BlobPosition(1, 0), "1", "ab1de", 0},
725725
{L_, "ab|cde", BlobPosition(1, 0), "12", "ab12e", 0},
726726
{L_, "ab|cde", BlobPosition(1, 0), "123", "ab123", 0},
727-
{L_, "ab|cde", BlobPosition(1, 0), "1234", "abcde", -31},
727+
{L_, "ab|cde", BlobPosition(1, 0), "1234", "abcde", -21},
728728
{L_, "ab|cde", BlobPosition(1, 1), "1", "abc1e", 0},
729729
{L_, "ab|cde", BlobPosition(1, 1), "12", "abc12", 0},
730-
{L_, "ab|cde", BlobPosition(1, 1), "123", "abcde", -31},
730+
{L_, "ab|cde", BlobPosition(1, 1), "123", "abcde", -21},
731731
{L_, "ab|cde|fg|h|ij", BlobPosition(1, 1), "123456", "abc123456j", 0}};
732732

733733
const size_t k_NUM_DATA = sizeof(k_DATA) / sizeof(*k_DATA);
@@ -1090,7 +1090,7 @@ static void test14_readUpToNBytes()
10901090
{L_, bmqu::BlobPosition(3, 0), 3, 3, "ghi"},
10911091
{L_, bmqu::BlobPosition(3, 0), 12, 3, "ghi"},
10921092
{L_, bmqu::BlobPosition(4, 0), 0, 0, ""},
1093-
{L_, bmqu::BlobPosition(4, 0), 1, -2, ""}, // invalid length
1093+
{L_, bmqu::BlobPosition(4, 0), 1, 0, ""},
10941094
{L_, bmqu::BlobPosition(5, 1), 0, -1, ""}, // invalid start
10951095
};
10961096

@@ -1105,18 +1105,18 @@ static void test14_readUpToNBytes()
11051105

11061106
char buffer[32];
11071107

1108-
int rc = bmqu::BlobUtil::readUpToNBytes(buffer,
1109-
blob,
1110-
test.d_start,
1111-
test.d_length);
1108+
int numRead = bmqu::BlobUtil::readUpToNBytes(buffer,
1109+
blob,
1110+
test.d_start,
1111+
test.d_length);
11121112

1113-
BMQTST_ASSERT_EQ_D("line " << test.d_line, rc, test.d_expectedRc);
1113+
BMQTST_ASSERT_EQ_D("line " << test.d_line, numRead, test.d_expectedRc);
11141114

11151115
if (test.d_expectedRc == 0) {
11161116
BMQTST_ASSERT_EQ_D(
11171117
"line " << test.d_line,
11181118
bsl::string(buffer,
1119-
test.d_length,
1119+
numRead,
11201120
bmqtst::TestHelperUtil::allocator()),
11211121
bsl::string(test.d_expected,
11221122
bmqtst::TestHelperUtil::allocator()));
@@ -1177,8 +1177,7 @@ static void test15_readNBytes()
11771177
{L_, bmqu::BlobPosition(3, 0), 3, 0, "ghi"},
11781178
{L_, bmqu::BlobPosition(3, 0), 12, -2, "ghi"},
11791179
{L_, bmqu::BlobPosition(4, 0), 0, 0, ""},
1180-
{L_, bmqu::BlobPosition(4, 0), 1, -21, ""},
1181-
// invalid length
1180+
{L_, bmqu::BlobPosition(4, 0), 1, -2, ""},
11821181
{L_, bmqu::BlobPosition(5, 1), 0, -11, ""},
11831182
// invalid start
11841183
};
@@ -1251,7 +1250,7 @@ static void test16_appendToBlob()
12511250
} k_DATA[] = {
12521251
{L_, "ab|cdXX", "ef|ghXX", 1, 2, 1, -1, "ab|cdXX"},
12531252
// invalid start
1254-
{L_, "ab|cdXX", "ef|ghXX", 1, 1, 4, -32, "ab|cdXX"},
1253+
{L_, "ab|cdXX", "ef|ghXX", 1, 1, 4, -22, "ab|cdXX"},
12551254
// invalid length
12561255
{L_, "ab|cdXX", "ef|ghXX", 0, 1, 0, 0, "ab|cd"},
12571256
{L_, "ab|cdXX", "ef|ghXX", 0, 1, 1, 0, "ab|cd|f"},

0 commit comments

Comments
 (0)