Skip to content

Commit 76d972a

Browse files
jimwwalkerjimwwalker
committed
MB-67520: Change various "stat" function to use string_view
Remove the use of char*/int API, primarily because the use of int triggers narrowing warnings when we mix in std containers that track sizes with size_t. Change-Id: Id94e76dd2c67ed382e4e0ae20528901cd1ed93cf Reviewed-on: https://review.couchbase.org/c/kv_engine/+/230636 Reviewed-by: Paolo Cocchi <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent bd1f649 commit 76d972a

File tree

2 files changed

+43
-103
lines changed

2 files changed

+43
-103
lines changed

‎engines/ep/src/ep_engine.cc‎

Lines changed: 39 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -3893,8 +3893,7 @@ cb::engine_errc EventuallyPersistentEngine::doMemoryStats(
38933893
cb::engine_errc EventuallyPersistentEngine::doVBucketStats(
38943894
CookieIface& cookie,
38953895
const AddStatFn& add_stat,
3896-
const char* stat_key,
3897-
int nkey,
3896+
std::string_view stat_key,
38983897
VBucketStatsDetailLevel detail) {
38993898
class StatVBucketVisitor : public VBucketVisitor {
39003899
public:
@@ -3943,38 +3942,24 @@ cb::engine_errc EventuallyPersistentEngine::doVBucketStats(
39433942
return cb::engine_errc::would_block;
39443943
}
39453944

3946-
if (nkey > 16 && strncmp(stat_key, "vbucket-details", 15) == 0) {
3945+
if (stat_key.size() > 16 && stat_key.starts_with("vbucket-details")) {
39473946
Expects(detail == VBucketStatsDetailLevel::Full);
3948-
std::string vbid(&stat_key[16], nkey - 16);
3949-
uint16_t vbucket_id(0);
3950-
if (!safe_strtous(vbid, vbucket_id)) {
3951-
return cb::engine_errc::invalid_arguments;
3952-
}
3953-
Vbid vbucketId = Vbid(vbucket_id);
3954-
VBucketPtr vb = getVBucket(vbucketId);
3955-
if (!vb) {
3956-
return cb::engine_errc::not_my_vbucket;
3947+
auto [status, vb] = getValidVBucketFromString(stat_key.substr(16));
3948+
if (status != cb::engine_errc::success) {
3949+
return status;
39573950
}
3958-
39593951
StatVBucketVisitor::addVBStats(cookie,
39603952
add_stat,
39613953
*vb,
39623954
kvBucket.get(),
39633955
VBucketStatsDetailLevel::Full);
3964-
} else if (nkey > 25 &&
3965-
strncmp(stat_key, "vbucket-durability-state", 24) == 0) {
3956+
} else if (stat_key.size() > 25 &&
3957+
stat_key.starts_with("vbucket-durability-state")) {
39663958
Expects(detail == VBucketStatsDetailLevel::Durability);
3967-
std::string vbid(&stat_key[25], nkey - 25);
3968-
uint16_t vbucket_id(0);
3969-
if (!safe_strtous(vbid, vbucket_id)) {
3970-
return cb::engine_errc::invalid_arguments;
3971-
}
3972-
Vbid vbucketId = Vbid(vbucket_id);
3973-
VBucketPtr vb = getVBucket(vbucketId);
3974-
if (!vb) {
3975-
return cb::engine_errc::not_my_vbucket;
3959+
auto [status, vb] = getValidVBucketFromString(stat_key.substr(25));
3960+
if (status != cb::engine_errc::success) {
3961+
return status;
39763962
}
3977-
39783963
StatVBucketVisitor::addVBStats(cookie,
39793964
add_stat,
39803965
*vb,
@@ -4273,25 +4258,18 @@ class StatCheckpointVisitor : public VBucketVisitor {
42734258
cb::engine_errc EventuallyPersistentEngine::doCheckpointStats(
42744259
CookieIface& cookie,
42754260
const AddStatFn& add_stat,
4276-
const char* stat_key,
4277-
int nkey) {
4278-
if (nkey == 10) {
4261+
std::string_view stat_key) {
4262+
if (stat_key.size() == 10) {
42794263
TRACE_EVENT0("ep-engine/task", "StatsCheckpoint");
42804264
auto* kvbucket = getKVBucket();
42814265
StatCheckpointVisitor scv(kvbucket, cookie, add_stat);
42824266
kvbucket->visit(scv);
42834267
return cb::engine_errc::success;
42844268
}
4285-
if (nkey > 11) {
4286-
std::string vbid(&stat_key[11], nkey - 11);
4287-
uint16_t vbucket_id(0);
4288-
if (!safe_strtous(vbid, vbucket_id)) {
4289-
return cb::engine_errc::invalid_arguments;
4290-
}
4291-
Vbid vbucketId = Vbid(vbucket_id);
4292-
VBucketPtr vb = getVBucket(vbucketId);
4293-
if (!vb) {
4294-
return cb::engine_errc::not_my_vbucket;
4269+
if (stat_key.size() > 11) {
4270+
auto [status, vb] = getValidVBucketFromString(stat_key.substr(11));
4271+
if (status != cb::engine_errc::success) {
4272+
return status;
42954273
}
42964274
StatCheckpointVisitor::addCheckpointStat(
42974275
cookie, add_stat, kvBucket.get(), *vb);
@@ -4303,29 +4281,20 @@ cb::engine_errc EventuallyPersistentEngine::doCheckpointStats(
43034281
cb::engine_errc EventuallyPersistentEngine::doDurabilityMonitorStats(
43044282
CookieIface& cookie,
43054283
const AddStatFn& add_stat,
4306-
const char* stat_key,
4307-
int nkey) {
4284+
std::string_view stat_key) {
43084285
const uint8_t size = 18; // size of "durability-monitor"
4309-
if (nkey == size) {
4286+
if (stat_key.size() == size) {
43104287
// Case stat_key = "durability-monitor"
43114288
// @todo: Return aggregated stats for all VBuckets.
43124289
// Implement as async, we don't what to block for too long.
43134290
return cb::engine_errc::not_supported;
43144291
}
4315-
if (nkey > size + 1) {
4292+
if (stat_key.size() > size + 1) {
43164293
// Case stat_key = "durability-monitor <vbid>"
4317-
const uint16_t vbidPos = size + 1;
4318-
std::string vbid_(&stat_key[vbidPos], nkey - vbidPos);
4319-
uint16_t vbid(0);
4320-
if (!safe_strtous(vbid_, vbid)) {
4321-
return cb::engine_errc::invalid_arguments;
4322-
}
4323-
4324-
VBucketPtr vb = getVBucket(Vbid(vbid));
4325-
if (!vb) {
4326-
// @todo: I would return an error code, but just replicating the
4327-
// behaviour of other stats for now
4328-
return cb::engine_errc::success;
4294+
auto [status, vb] =
4295+
getValidVBucketFromString(stat_key.substr(size + 1));
4296+
if (status != cb::engine_errc::success) {
4297+
return status;
43294298
}
43304299
vb->addDurabilityMonitorStats(add_stat, cookie);
43314300
}
@@ -5129,9 +5098,7 @@ EventuallyPersistentEngine::getValidVBucketFromString(std::string_view vbNum) {
51295098
return {cb::engine_errc::invalid_arguments, {}};
51305099
}
51315100
uint16_t vbucket_id;
5132-
5133-
std::string vbNumString(vbNum);
5134-
if (!safe_strtous(vbNumString, vbucket_id)) {
5101+
if (!safe_strtous(vbNum, vbucket_id)) {
51355102
return {cb::engine_errc::invalid_arguments, {}};
51365103
}
51375104
Vbid vbid = Vbid(vbucket_id);
@@ -5145,29 +5112,20 @@ EventuallyPersistentEngine::getValidVBucketFromString(std::string_view vbNum) {
51455112
cb::engine_errc EventuallyPersistentEngine::doSeqnoStats(
51465113
CookieIface& cookie,
51475114
const AddStatFn& add_stat,
5148-
const char* stat_key,
5149-
int nkey) {
5115+
std::string_view stat_key) {
51505116
if (getKVBucket()->maybeWaitForVBucketWarmup(&cookie)) {
51515117
return cb::engine_errc::would_block;
51525118
}
51535119

5154-
if (nkey > 14) {
5155-
std::string value(stat_key + 14, nkey - 14);
5156-
5157-
try {
5158-
checkNumeric(value.c_str());
5159-
} catch(std::runtime_error &) {
5160-
return cb::engine_errc::invalid_arguments;
5120+
if (stat_key.size() > 14) {
5121+
auto [status, vb] = getValidVBucketFromString(stat_key.substr(14));
5122+
if (status != cb::engine_errc::success) {
5123+
return status;
51615124
}
5162-
5163-
Vbid vbucket(atoi(value.c_str()));
5164-
VBucketPtr vb = getVBucket(vbucket);
5165-
if (!vb || vb->getState() == vbucket_state_dead) {
5125+
if (vb->getState() == vbucket_state_dead) {
51665126
return cb::engine_errc::not_my_vbucket;
51675127
}
5168-
51695128
addSeqnoVbStats(cookie, add_stat, vb);
5170-
51715129
return cb::engine_errc::success;
51725130
}
51735131

@@ -5614,46 +5572,32 @@ cb::engine_errc EventuallyPersistentEngine::getStats(
56145572
return doHashStats(c, add_stat);
56155573
}
56165574
if (key == "vbucket"sv) {
5617-
return doVBucketStats(c,
5618-
add_stat,
5619-
key.data(),
5620-
key.size(),
5621-
VBucketStatsDetailLevel::State);
5575+
return doVBucketStats(c, add_stat, key, VBucketStatsDetailLevel::State);
56225576
}
56235577
if (key == "prev-vbucket"sv) {
5624-
return doVBucketStats(c,
5625-
add_stat,
5626-
key.data(),
5627-
key.size(),
5628-
VBucketStatsDetailLevel::PreviousState);
5578+
return doVBucketStats(
5579+
c, add_stat, key, VBucketStatsDetailLevel::PreviousState);
56295580
}
56305581
if (key.starts_with("vbucket-durability-state")) {
5631-
return doVBucketStats(c,
5632-
add_stat,
5633-
key.data(),
5634-
key.size(),
5635-
VBucketStatsDetailLevel::Durability);
5582+
return doVBucketStats(
5583+
c, add_stat, key, VBucketStatsDetailLevel::Durability);
56365584
}
56375585
if (key.starts_with("vbucket-details")) {
5638-
return doVBucketStats(c,
5639-
add_stat,
5640-
key.data(),
5641-
key.size(),
5642-
VBucketStatsDetailLevel::Full);
5586+
return doVBucketStats(c, add_stat, key, VBucketStatsDetailLevel::Full);
56435587
}
56445588
if (key.starts_with("vbucket-seqno")) {
5645-
return doSeqnoStats(c, add_stat, key.data(), key.size());
5589+
return doSeqnoStats(c, add_stat, key);
56465590
}
56475591

56485592
if (key == "encryption-key-ids") {
56495593
return doEncryptionKeyIdsStats(c, add_stat);
56505594
}
56515595

56525596
if (key.starts_with("checkpoint")) {
5653-
return doCheckpointStats(c, add_stat, key.data(), key.size());
5597+
return doCheckpointStats(c, add_stat, key);
56545598
}
56555599
if (key.starts_with("durability-monitor")) {
5656-
return doDurabilityMonitorStats(c, add_stat, key.data(), key.size());
5600+
return doDurabilityMonitorStats(c, add_stat, key);
56575601
}
56585602
if (key == "timings"sv) {
56595603
doTimingStats(bucketCollector);

‎engines/ep/src/ep_engine.h‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,8 +1263,7 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
12631263
const AddStatFn& add_stat);
12641264
cb::engine_errc doVBucketStats(CookieIface& cookie,
12651265
const AddStatFn& add_stat,
1266-
const char* stat_key,
1267-
int nkey,
1266+
std::string_view,
12681267
VBucketStatsDetailLevel detail);
12691268
cb::engine_errc doEncryptionKeyIdsStats(CookieIface& cookie,
12701269
const AddStatFn& add_stat);
@@ -1274,15 +1273,13 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
12741273
std::string_view keyArgs);
12751274
cb::engine_errc doCheckpointStats(CookieIface& cookie,
12761275
const AddStatFn& add_stat,
1277-
const char* stat_key,
1278-
int nkey);
1276+
std::string_view);
12791277
cb::engine_errc doCheckpointDump(CookieIface& cookie,
12801278
const AddStatFn& addStat,
12811279
std::string_view keyArgs);
12821280
cb::engine_errc doDurabilityMonitorStats(CookieIface& cookie,
12831281
const AddStatFn& add_stat,
1284-
const char* stat_key,
1285-
int nkey);
1282+
std::string_view);
12861283
cb::engine_errc doDurabilityMonitorDump(CookieIface& cookie,
12871284
const AddStatFn& addStat,
12881285
std::string_view keyArgs);
@@ -1335,8 +1332,7 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
13351332
const AddStatFn& add_stat);
13361333
cb::engine_errc doSeqnoStats(CookieIface& cookie,
13371334
const AddStatFn& add_stat,
1338-
const char* stat_key,
1339-
int nkey);
1335+
std::string_view);
13401336
void addSeqnoVbStats(CookieIface& cookie,
13411337
const AddStatFn& add_stat,
13421338
const VBucketPtr& vb);

0 commit comments

Comments
 (0)