Skip to content

Commit e299de9

Browse files
clang-tidy: Fix various C++ errors
- src/v/utils/retry_chain_node.h — Fixed bugprone-use-after-move: format message once upfront instead of double-forwarding args. - src/v/cluster/partition_leaders_table.h — Fixed bugprone-use-after-move: capture f by reference instead of forwarding into a lambda inside a loop. - src/v/test_utils/randoms.h — Fixed bugprone-use-after-move: don't forward args in a loop, just pass them as lvalues.
1 parent 7b2e905 commit e299de9

5 files changed

Lines changed: 18 additions & 23 deletions

File tree

src/v/cluster/inventory_service.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,7 @@ default_leaders_provider::ntps(ss::abort_source& as) {
116116
std::exception_ptr ep;
117117
try {
118118
co_await _leaders_table.local().for_each_leader(
119-
[&ntps,
120-
self_node_id](auto tp_ns, auto pid, auto node_id, auto) mutable {
119+
[&ntps, self_node_id](auto tp_ns, auto pid, auto node_id, auto) {
121120
if (node_id == self_node_id) {
122121
ntps.insert(model::ntp{tp_ns.ns, tp_ns.tp, pid});
123122
}

src/v/cluster/metadata_dissemination_handler.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ make_get_leadership_reply(partition_leaders_table& leaders) {
8585
model::topic_namespace_view tp_ns,
8686
model::partition_id pid,
8787
std::optional<model::node_id> leader,
88-
model::term_id term) mutable {
88+
model::term_id term) {
8989
ret.emplace_back(model::ntp(tp_ns.ns, tp_ns.tp, pid), term, leader);
9090
});
9191
co_return get_leadership_reply{

src/v/cluster/partition_leaders_table.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ class partition_leaders_table {
7878
model::term_id term) {
7979
{ f(tp_ns, pid, leader, term) } -> std::same_as<void>;
8080
}
81-
ss::future<> for_each_leader(Func&& f) {
81+
ss::future<> for_each_leader(const Func& f) {
8282
auto holder = _gate.hold();
8383
auto u = co_await _mutex.get_units();
8484
ssx::async_counter counter;
@@ -87,8 +87,7 @@ class partition_leaders_table {
8787
counter,
8888
partition_leaders.begin(),
8989
partition_leaders.end(),
90-
[&tp_ns, f = std::forward<Func>(f)](
91-
const partition_leaders::value_type& p) mutable {
90+
[&tp_ns, &f](const partition_leaders::value_type& p) {
9291
f(tp_ns,
9392
model::partition_id(p.first),
9493
p.second.current_leader,

src/v/test_utils/randoms.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,25 +83,26 @@ auto random_chunked_vector(Fn&& gen, size_t size = 20) -> chunked_vector<T> {
8383
template<
8484
typename Fn,
8585
typename... Args,
86-
typename T = std::invoke_result_t<Fn, Args...>>
87-
inline auto random_frag_vector(Fn&& gen, size_t size = 20, Args&&... args)
86+
typename T = std::invoke_result_t<Fn, Args&&...>>
87+
inline auto random_frag_vector(const Fn& gen, size_t size = 20, Args&&... args)
8888
-> chunked_vector<T> {
8989
chunked_vector<T> v;
9090
while (size-- > 0) {
91-
v.push_back(gen(std::forward<Args>(args)...));
91+
v.push_back(gen(args...));
9292
}
9393
return v;
9494
}
9595

9696
template<
9797
typename Fn,
9898
typename... Args,
99-
typename T = std::invoke_result_t<Fn, Args...>>
100-
inline auto random_chunked_vector(Fn&& gen, size_t size = 20, Args&&... args)
99+
typename T = std::invoke_result_t<Fn, const Args&...>>
100+
inline auto
101+
random_chunked_vector(Fn&& gen, size_t size = 20, const Args&... args)
101102
-> chunked_vector<T> {
102103
chunked_vector<T> v;
103104
while (size-- > 0) {
104-
v.push_back(gen(std::forward<Args>(args)...));
105+
v.push_back(gen(args...));
105106
}
106107
return v;
107108
}

src/v/utils/retry_chain_node.h

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -634,23 +634,19 @@ class basic_retry_chain_logger final {
634634
void
635635
log(ss::log_level lvl, fmt::format_string<Args...> format, Args&&... args)
636636
const {
637-
std::optional<ss::sstring> trace;
637+
if (!_has_tracing && !_log.is_enabled(lvl)) {
638+
return;
639+
}
640+
auto msg = ssx::sformat(format, std::forward<Args>(args)...);
638641
if (_has_tracing) {
639-
trace = ssx::sformat(format, std::forward<Args>(args)...);
640-
_node.maybe_add_trace(trace.value());
642+
_node.maybe_add_trace(msg);
641643
}
642644
if (_log.is_enabled(lvl)) {
643645
auto lambda = [&](ss::logger& logger, ss::log_level lvl) {
644-
auto msg = trace.value_or(
645-
ssx::sformat(format, std::forward<Args>(args)...));
646646
if (_ctx) {
647-
logger.log(
648-
lvl,
649-
"{} - {}",
650-
_node("{}", _ctx.value()),
651-
std::move(msg));
647+
logger.log(lvl, "{} - {}", _node("{}", _ctx.value()), msg);
652648
} else {
653-
logger.log(lvl, "{} - {}", _node(), std::move(msg));
649+
logger.log(lvl, "{} - {}", _node(), msg);
654650
}
655651
};
656652
do_log(lvl, std::move(lambda));

0 commit comments

Comments
 (0)