bryancall opened a new pull request, #13591: URL: https://github.com/apache/trafficserver/pull/13591
Static analysis flagged a set of unnecessary copies. This fixes the ones that are real and leaves the ones that are not. ### What changed - **`std::move` on a local's last use** in `plugins/header_rewrite/parser.cc`, `plugins/cachekey/pattern.cc`, and `plugins/experimental/rate_limit/limiter.h`, where a local `std::string` (or an element of a local vector that is about to be destroyed) was copied into a container or into a by-value parameter. - **`auto` to `auto const &`** in `src/proxy/http/remap/NextHopSelectionStrategy.cc` and `NextHopConsistentHash.cc`, where `YAML::Node::Scalar()` already returns `const std::string &` and the result was being copied into a local that is only read. 15 changes across 5 files. ### Why these and not the others Every site was checked individually rather than applied in bulk, and most of the reported findings were left alone: - The moved-from objects are never read again. In `parser.cc` the `tokens` vector is a by-value parameter (the caller deliberately passes a copy), so moving out of its elements cannot be observed by the caller. - The const-reference binds do not dangle. `Node::Scalar()` returns a reference into `detail::node` storage owned by the document's shared memory holder, not into the temporary `Node` handle returned by `operator[]`, and the `Map &` parameter keeps that holder alive for the whole function. - Findings in test helpers were skipped: making a test copy one fewer string is not worth the churn. - Findings where the "copy" is a `std::string_view`, or where the expression returns by value so a const-reference bind would remove no copy at all, were skipped as false positives. A larger group of `Big parameter passed by value` findings on this same code was deliberately **not** addressed. Those are almost entirely `ConfigContext` and `YAML::Node`, which are reference-counted handles that a size-based heuristic flags by `sizeof`. `ConfigContext` documents at its declaration that copies are intentional and that move is suppressed so `std::move` silently copies, which `execute_reload()` depends on; `const YAML::Node &` would also change `operator[]` semantics, since the const overload does not create missing keys. Converting them would risk a silent config-parsing change for no measurable gain on a path that runs a handful of times per reload. ### Testing Builds clean with experimental plugins enabled. All 166 unit tests pass. -- 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]
