Re: [PR] Remove unnecessary copies flagged by static analysis [trafficserver]

["bryancall (via GitHub)" <gi...@apache.org>]

bryancall commented on code in PR #13591:
URL: https://github.com/apache/trafficserver/pull/13591#discussion_r3926951736


##########
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:
   Fixed. #13601 landed in the meantime and corrected this same line on master, so I merged master in and re-applied the moves on the corrected argument order:
   
   ```cpp
   initializeMetrics(RATE_LIMITER_TYPE_SNI, std::move(tag), std::move(prefix));
   ```
   
   That matches the declaration at `limiter.h:191`, `initializeMetrics(uint type, std::string tag, std::string prefix)`. Both parameters are by-value `std::string`, and the `Dbg()` call above reads both before the move, so the moves are still worth having.



##########
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:
   Fixed. The description was written when this branch still carried the five `auto` to `auto const &` conversions in `NextHopSelectionStrategy.cc` and `NextHopConsistentHash.cc`. Commit 1f50277f8 reverted all five, so the diff has been three files for a while and the description had not caught up.
   
   I have rewritten it to match the actual diff: 8 changes across 3 files, with a section explaining why the NextHop conversions were dropped rather than the old text arguing they were safe to keep.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org