Copilot commented on code in PR #13591:
URL: https://github.com/apache/trafficserver/pull/13591#discussion_r3899014633
##########
plugins/experimental/rate_limit/limiter.h:
##########
@@ -225,7 +225,7 @@ template <class T> class RateLimiter
std::string tag = metrics["tag"] ? metrics["tag"].as<std::string>() :
name();
Dbg(dbg_ctl, "Metrics for selector rule: %s(%s, %s)", name().c_str(),
prefix.c_str(), tag.c_str());
- initializeMetrics(RATE_LIMITER_TYPE_SNI, prefix, tag);
+ initializeMetrics(RATE_LIMITER_TYPE_SNI, std::move(prefix),
std::move(tag));
Review Comment:
The arguments to initializeMetrics() appear to be passed in the wrong order.
The signature is initializeMetrics(uint type, std::string tag, std::string
prefix), but this call passes prefix first and tag second, which will swap the
metric tag/prefix values.
##########
plugins/header_rewrite/parser.cc:
##########
@@ -228,7 +228,7 @@ Parser::preprocess(std::vector<std::string> tokens)
_arg = tokens[1] + tokens[2];
} else if (tokens.size() > 1) {
// This is for the regular expression, which for some reason has its
own handling?? ToDo: Why ?
- _arg = tokens[1];
+ _arg = std::move(tokens[1]);
Review Comment:
The PR description says copies from YAML::Node::Scalar() were removed by
switching `auto` to `auto const &` in NextHopSelectionStrategy.cc and
NextHopConsistentHash.cc, but those files still appear to use `auto <name> =
...Scalar();` (which copies) and no `auto const & ... = ...Scalar();` bindings
are present. Either the description needs updating, or the intended changes to
those files are missing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
