Skip to content

chore: reduce boilerplate by removing redundant function declarations #6207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
romange wants to merge 1 commit into
base: main
Choose a base branch
from
+78 −141
Open

chore: reduce boilerplate by removing redundant function declarations #6207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 26 additions & 26 deletions src/server/string_family.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class SetCmd {
bool explicit_journal_; // call RecordJournal (auto journaling disabled)
};

size_t SetRange(std::string* value, size_t start, std::string_view range) {
size_t SetRangeInternal(std::string* value, size_t start, std::string_view range) {
value->resize(max(value->size(), start + range.size()));
memcpy(value->data() + start, range.data(), range.size());
return value->size();
Expand Down Expand Up @@ -157,15 +157,15 @@ OpResult<TResultOrT<size_t>> OpSetRange(const OpArgs& op_args, string_view key,
return {op_args.shard->tiered_storage()->Modify<size_t>(
op_args.db_cntx.db_index, key, res.it->second,
[start = start, range = string(range)](std::string* s) {
return SetRange(s, start, range);
return SetRangeInternal(s, start, range);
})};
} else {
string value;

if (!res.is_new)
value = res.it->second.ToString();

size_t len = SetRange(&value, start, range);
size_t len = SetRangeInternal(&value, start, range);
res.it->second.SetValue(value);
return {len};
}
Expand Down Expand Up @@ -984,7 +984,7 @@ OpStatus SetCmd::CachePrevIfNeeded(const SetCmd::SetParams& params, DbSlice::Ite
return OpStatus::OK;
}

void StringFamily::Set(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdSet(CmdArgList args, const CommandContext& cmnd_cntx) {
Copy link

@augmentcode augmentcode bot Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Cmd* handlers are now free functions outside the existing anonymous namespace, so they get external linkage. Consider moving them into the anonymous namespace (or making them static) to avoid exporting many Cmd* symbols and potential ODR/link collisions (also applies to the other Cmd* handlers in this file).

🤖 Was this useful? React with 👍 or 👎

facade::CmdArgParser parser{args};

auto [key, value] = parser.Next<string_view, string_view>();
Expand Down Expand Up @@ -1092,7 +1092,7 @@ void StringFamily::Set(CmdArgList args, const CommandContext& cmnd_cntx) {
}

/// (P)SETEX key seconds (milliseconds) value
void StringFamily::SetExGeneric(CmdArgList args, const CommandContext& cmd_cntx) {
void CmdSetExGeneric(CmdArgList args, const CommandContext& cmd_cntx) {
string_view cmd_name = cmd_cntx.cid->name();

CmdArgParser parser{args};
Expand Down Expand Up @@ -1122,7 +1122,7 @@ void StringFamily::SetExGeneric(CmdArgList args, const CommandContext& cmd_cntx)
builder->SendError(SetGeneric(sparams, key, value, cmd_cntx));
}

void StringFamily::SetNx(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdSetNx(CmdArgList args, const CommandContext& cmnd_cntx) {
string_view key = ArgS(args, 0);
string_view value = ArgS(args, 1);

Expand All @@ -1142,7 +1142,7 @@ void StringFamily::SetNx(CmdArgList args, const CommandContext& cmnd_cntx) {
}
}

void StringFamily::Get(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdGet(CmdArgList args, const CommandContext& cmnd_cntx) {
auto cb = [key = ArgS(args, 0)](Transaction* tx, EngineShard* es) -> OpResult<StringResult> {
auto it_res = tx->GetDbSlice(es->shard_id()).FindReadOnly(tx->GetDbContext(), key, OBJ_STRING);
if (!it_res.ok())
Expand All @@ -1154,7 +1154,7 @@ void StringFamily::Get(CmdArgList args, const CommandContext& cmnd_cntx) {
GetReplies{cmnd_cntx.rb}.Send(cmnd_cntx.tx->ScheduleSingleHopT(cb));
}

void StringFamily::GetDel(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdGetDel(CmdArgList args, const CommandContext& cmnd_cntx) {
auto cb = [key = ArgS(args, 0)](Transaction* tx, EngineShard* es) -> OpResult<StringResult> {
auto& db_slice = tx->GetDbSlice(es->shard_id());
auto it_res = db_slice.FindMutable(tx->GetDbContext(), key, OBJ_STRING);
Expand All @@ -1169,7 +1169,7 @@ void StringFamily::GetDel(CmdArgList args, const CommandContext& cmnd_cntx) {
GetReplies{cmnd_cntx.rb}.Send(cmnd_cntx.tx->ScheduleSingleHopT(cb));
}

void StringFamily::GetSet(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdGetSet(CmdArgList args, const CommandContext& cmnd_cntx) {
string_view key = ArgS(args, 0);
string_view value = ArgS(args, 1);

Expand All @@ -1182,15 +1182,15 @@ void StringFamily::GetSet(CmdArgList args, const CommandContext& cmnd_cntx) {
GetReplies{cmnd_cntx.rb}.Send(std::move(prev));
}

void StringFamily::Append(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdAppend(CmdArgList args, const CommandContext& cmnd_cntx) {
ExtendGeneric(args, false, cmnd_cntx.tx, cmnd_cntx.rb);
}

void StringFamily::Prepend(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdPrepend(CmdArgList args, const CommandContext& cmnd_cntx) {
ExtendGeneric(args, true, cmnd_cntx.tx, cmnd_cntx.rb);
}

void StringFamily::GetEx(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdGetEx(CmdArgList args, const CommandContext& cmnd_cntx) {
CmdArgParser parser{args};
string_view key = parser.Next();

Expand Down Expand Up @@ -1258,12 +1258,12 @@ void StringFamily::GetEx(CmdArgList args, const CommandContext& cmnd_cntx) {
GetReplies{cmnd_cntx.rb}.Send(cmnd_cntx.tx->ScheduleSingleHopT(cb));
}

void StringFamily::Incr(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdIncr(CmdArgList args, const CommandContext& cmnd_cntx) {
string_view key = ArgS(args, 0);
return IncrByGeneric(key, 1, cmnd_cntx.tx, cmnd_cntx.rb);
}

void StringFamily::IncrBy(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdIncrBy(CmdArgList args, const CommandContext& cmnd_cntx) {
string_view key = ArgS(args, 0);
string_view sval = ArgS(args, 1);
int64_t val;
Expand All @@ -1274,7 +1274,7 @@ void StringFamily::IncrBy(CmdArgList args, const CommandContext& cmnd_cntx) {
return IncrByGeneric(key, val, cmnd_cntx.tx, cmnd_cntx.rb);
}

void StringFamily::IncrByFloat(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdIncrByFloat(CmdArgList args, const CommandContext& cmnd_cntx) {
string_view key = ArgS(args, 0);
string_view sval = ArgS(args, 1);
double val;
Expand All @@ -1298,12 +1298,12 @@ void StringFamily::IncrByFloat(CmdArgList args, const CommandContext& cmnd_cntx)
rb->SendDouble(result.value());
}

void StringFamily::Decr(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdDecr(CmdArgList args, const CommandContext& cmnd_cntx) {
string_view key = ArgS(args, 0);
return IncrByGeneric(key, -1, cmnd_cntx.tx, cmnd_cntx.rb);
}

void StringFamily::DecrBy(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdDecrBy(CmdArgList args, const CommandContext& cmnd_cntx) {
string_view key = ArgS(args, 0);
string_view sval = ArgS(args, 1);
int64_t val;
Expand Down Expand Up @@ -1340,7 +1340,7 @@ void ReorderShardResults(absl::Span<MGetResponse> mget_resp, const Transaction*
}
}

void StringFamily::MGet(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdMGet(CmdArgList args, const CommandContext& cmnd_cntx) {
DCHECK_GE(args.size(), 1U);

uint8_t fetch_mask = 0;
Expand Down Expand Up @@ -1398,7 +1398,7 @@ void StringFamily::MGet(CmdArgList args, const CommandContext& cmnd_cntx) {
}
}

void StringFamily::MSet(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdMSet(CmdArgList args, const CommandContext& cmnd_cntx) {
if (VLOG_IS_ON(2)) {
string str;
for (size_t i = 1; i < args.size(); ++i) {
Expand All @@ -1425,7 +1425,7 @@ void StringFamily::MSet(CmdArgList args, const CommandContext& cmnd_cntx) {
}
}

void StringFamily::MSetNx(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdMSetNx(CmdArgList args, const CommandContext& cmnd_cntx) {
atomic_bool exists{false};

auto cb = [&](Transaction* t, EngineShard* es) {
Expand Down Expand Up @@ -1462,14 +1462,14 @@ void StringFamily::MSetNx(CmdArgList args, const CommandContext& cmnd_cntx) {
cmnd_cntx.rb->SendLong(to_skip || (*result != OpStatus::OK) ? 0 : 1);
}

void StringFamily::StrLen(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdStrLen(CmdArgList args, const CommandContext& cmnd_cntx) {
auto cb = [key = ArgS(args, 0)](Transaction* t, EngineShard* shard) {
return OpStrLen(t->GetOpArgs(shard), key);
};
GetReplies{cmnd_cntx.rb}.Send(cmnd_cntx.tx->ScheduleSingleHopT(cb));
}

void StringFamily::GetRange(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdGetRange(CmdArgList args, const CommandContext& cmnd_cntx) {
CmdArgParser parser(args);
auto [key, start, end] = parser.Next<string_view, int32_t, int32_t>();

Expand All @@ -1484,7 +1484,7 @@ void StringFamily::GetRange(CmdArgList args, const CommandContext& cmnd_cntx) {
GetReplies{cmnd_cntx.rb}.Send(cmnd_cntx.tx->ScheduleSingleHopT(cb));
}

void StringFamily::SetRange(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdSetRange(CmdArgList args, const CommandContext& cmnd_cntx) {
CmdArgParser parser(args);
auto [key, start, value] = parser.Next<string_view, int32_t, string_view>();
auto* builder = cmnd_cntx.rb;
Expand Down Expand Up @@ -1520,7 +1520,7 @@ void StringFamily::SetRange(CmdArgList args, const CommandContext& cmnd_cntx) {
* 5. The number of seconds until the limit will reset to its maximum capacity.
* Equivalent to X-RateLimit-Reset.
*/
void StringFamily::ClThrottle(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdClThrottle(CmdArgList args, const CommandContext& cmnd_cntx) {
constexpr uint64_t kSecondToNanoSecond = 1000000000;
const string_view key = ArgS(args, 0);

Expand Down Expand Up @@ -1627,7 +1627,7 @@ void StringFamily::ClThrottle(CmdArgList args, const CommandContext& cmnd_cntx)

// Implements the memcache GAT command. The expected input is
// GAT <expiry-in-seconds> key [keys...]
void StringFamily::GAT(CmdArgList args, const CommandContext& cmnd_cntx) {
void CmdGAT(CmdArgList args, const CommandContext& cmnd_cntx) {
DCHECK_GE(args.size(), 1U);

auto* builder = cmnd_cntx.rb;
Expand Down Expand Up @@ -1680,7 +1680,7 @@ void StringFamily::GAT(CmdArgList args, const CommandContext& cmnd_cntx) {
rb->SendGetEnd();
}

#define HFUNC(x) SetHandler(&StringFamily::x)
#define HFUNC(x) SetHandler(&Cmd##x)

void StringFamily::Register(CommandRegistry* registry) {
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name should be StringFamily::Register to match the declaration in the header file (string_family.h:20), not CmdRegister. This will cause a linker error because StringFamily::Register is declared but not defined, while CmdRegister is defined but never called. The function is invoked as StringFamily::Register(&registry_) in main_service.cc:3060.

Copilot uses AI. Check for mistakes.
constexpr uint32_t kMSetMask =
Expand Down
29 changes: 0 additions & 29 deletions src/server/string_family.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,6 @@ class CommandRegistry;
class StringFamily {
public:
static void Register(CommandRegistry* registry);

private:
using SinkReplyBuilder = facade::SinkReplyBuilder;
using CmdArgList = facade::CmdArgList;

static void Append(CmdArgList args, const CommandContext& cmnd_cntx);
static void Decr(CmdArgList args, const CommandContext& cmnd_cntx);
static void DecrBy(CmdArgList args, const CommandContext& cmnd_cntx);
static void Get(CmdArgList args, const CommandContext& cmnd_cntx);
static void GetDel(CmdArgList args, const CommandContext& cmnd_cntx);
static void GetRange(CmdArgList args, const CommandContext& cmnd_cntx);
static void GetSet(CmdArgList args, const CommandContext& cmnd_cntx);
static void GetEx(CmdArgList args, const CommandContext& cmnd_cntx);
static void Incr(CmdArgList args, const CommandContext& cmnd_cntx);
static void IncrBy(CmdArgList args, const CommandContext& cmnd_cntx);
static void IncrByFloat(CmdArgList args, const CommandContext& cmnd_cntx);
static void MGet(CmdArgList args, const CommandContext& cmnd_cntx);
static void MSet(CmdArgList args, const CommandContext& cmnd_cntx);
static void MSetNx(CmdArgList args, const CommandContext& cmnd_cntx);

static void Set(CmdArgList args, const CommandContext& cmnd_cntx);
static void SetNx(CmdArgList args, const CommandContext& cmnd_cntx);
static void SetRange(CmdArgList args, const CommandContext& cmnd_cntx);
static void SetExGeneric(CmdArgList args, const CommandContext& cmnd_cntx);
static void StrLen(CmdArgList args, const CommandContext& cmnd_cntx);
static void Prepend(CmdArgList args, const CommandContext& cmnd_cntx);

static void ClThrottle(CmdArgList args, const CommandContext& cmnd_cntx);
static void GAT(CmdArgList args, const CommandContext& cmnd_cntx);
};

} // namespace dfly
Loading
Loading