Avoid needless copies reported by Coverity Scan

[bryancall]

Part 1 of 3 splitting a Coverity Scan cleanup into independently reviewable pieces. This one is deliberately the boring part: no behavior change anywhere.

What this does

• Replaces a copy with std::move where the source is never used again (42 sites).
• Binds const auto & / auto const & instead of copying where a loop variable or local only reads the referent (7 sites).
• Adds <utility> to four files that name std::move without including it directly.

How it was checked

Every move source was traced to the end of its scope to confirm it is not read after the move. The three enable_inbound_connection_tracking(std::move(conn_track_group)) sites are worth a second look if you want a spot check: conn_track_group is declared inside each accept loop body, so no iteration inherits a moved-from group. A shared declaration there would have silently disabled inbound connection tracking after the first connection.

Every reference conversion was checked to make sure it binds to something that outlives the use, not to a temporary.

Reports deliberately not acted on

Coverity flags five auto copies in the next-hop YAML parsers (NextHopSelectionStrategy.cc, NextHopConsistentHash.cc). Those are false positives and are left alone: the node accessors return by value, so const auto &x = n["scheme"].Scalar() binds a reference into a temporary that dies at the end of the statement. GCC's -Wdangling-reference confirms it.

ConfigContext parameters reported as oversized are also left alone. They are by value by design, because the reload handler signature requires it and the handlers mutate the context.

Verification

Clean build with no new warnings and the full unit test suite passing (137/137) on Fedora, GCC 16.1.1.

Getting every modified file actually compiled took three extra options, which is worth stating precisely rather than claiming full coverage:

• uri_signing needs cjose, stek_share needs nuraft, and jax_fingerprint defaults to off. With -DENABLE_URI_SIGNING=ON -DENABLE_STEK_SHARE=ON -DENABLE_JAX_FINGERPRINT=ON all of them build and their objects appear in the graph.
• The two access_control changes sit behind #ifdef ACCESS_CONTROL_LOG_SECRETS, which no build here defines, so those two lines are reviewed but not compiled.

Draft while CI runs.

[Copilot]

Pull request overview

This PR is part of a larger Coverity Scan cleanup and focuses on eliminating unnecessary copies by switching to std::move where appropriate and binding references instead of copying values, with the stated intent of no behavior changes.

Changes:

• Replaces various local copies with moves when the source is not used again (e.g., push/insert into containers, assignments, parameter passing).
• Converts some range/loop and local variable copies to const auto & to avoid copying read-only values.
• Adds missing <utility> includes in several files that now directly use std::move.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/tsutil/Metrics.cc	Removes misleading std::move on a const & parameter when pushing into the derived-metrics list.
src/tscore/runroot.cc	Uses std::move for assigned runroot paths and map values; adds <utility>.
src/tscore/Layout.cc	Uses moves when transferring temporary strings into path/prefix.
src/tscore/ArgParser.cc	Moves lookup_key into the stored option record to avoid an extra copy.
src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h	Moves per-item decoded structs into the response list.
src/traffic_ctl/CtrlCommands.cc	Moves plugin message params into the request object.
src/proxy/http/PreWarmManager.cc	Moves config/shared objects into newly built reconfiguration map entries.
src/proxy/HostStatus.cc	Moves per-host status objects into the output vector.
src/iocore/net/UnixNetAccept.cc	Moves per-accept ConnectionTracker::Group into the VC to avoid shared_ptr refcount churn.
src/iocore/net/SSLUtils.cc	Moves generated certificate/key path strings and name sets into containers/variables.
src/iocore/net/SSLNetVConnection.cc	Moves the shared session pointer into the connection; adds <utility>.
src/iocore/net/SSLCertLookup.cc	Avoids copying secret policy names by iterating with const &.
src/config/ssl_multicert.cc	Moves result/errata in early returns to avoid unnecessary vector copies.
src/api/InkAPI.cc	Avoids an extra YAML::Node copy in TSRPCHandlerDone by binding a reference.
plugins/traffic_dump/transaction_data.cc	Avoids copying the stored HTTP version by binding a const &.
plugins/traffic_dump/session_data.cc	Moves log filename into session data; adds <utility>.
plugins/origin_server_auth/origin_server_auth.cc	Moves region into the map entry to avoid a copy.
plugins/header_rewrite/operators.cc	Avoids copying parser arg/value strings when initializing run-plugin.
plugins/experimental/stek_share/stek_share.cc	Moves shared_ptr/nuraft pointers into stored state and initialization calls.
plugins/experimental/stek_share/state_manager.h	Moves newly created server config pointers into the saved config list; adds <utility>.
plugins/experimental/stek_share/state_machine.h	Moves snapshot context into the stored snapshot pointer.
plugins/experimental/stek_share/log_store.cc	Moves cloned / serialized nuraft objects into containers/slots.
plugins/experimental/rate_limit/txn_limiter.cc	Moves tag/prefix into metrics initialization; adds <utility>.
plugins/experimental/rate_limit/sni_selector.cc	Moves alias strings into addAlias to avoid a copy.
plugins/experimental/jax_fingerprint/ja4h/test.cc	Avoids copying map entries in iteration by using const &.
plugins/experimental/access_control/pattern.cc	Moves captured strings into result vectors to avoid copies.
plugins/experimental/access_control/config.cc	Moves parsed secret values into containers and logs from the stored container value.
plugins/esi/lib/EsiParser.cc	Moves newly created nodes into node lists; adds <utility>.
plugins/cachekey/configs.cc	Avoids copying parsed key types by iterating with const &.
plugins/cachekey/cachekey.cc	Moves constructed header strings into the capture set.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bryancall]

This one is pre-existing rather than something this change introduces. The only edit here is path = env_path becoming path = std::move(env_path); the while (path.back() == '/') loop above is untouched upstream code.

You are right that it is a real problem though: with TS_ROOT set but empty, getenv returns a non-null empty string, so path is empty and path.back() is undefined. Since the goal of this pull request is to be provably free of behavior changes, I would rather not add a guard here and will raise it separately.

[bryancall]

Following up: a dedicated change for this is in progress, so it is not going to ride along here.

Two extra details worth recording while looking at it. There is a second route to the same problem that does not need an empty TS_ROOT at all: the loop pops until the string does not end in a slash, so a TS_ROOT of "/" or "///" empties the string partway through and then calls back() on it again. And src/tscore/runroot.cc has the same unguarded shape in get_parent_yaml_path(), where whole_path.back() is tested before any emptiness check, even though the loop three lines below it does check for empty.

The guard itself is one token, but the open question is what an empty or all-slashes TS_ROOT should mean. Falling through to the compile time prefix would silently ignore what the operator asked for, and every layout path is derived from this value, so I would rather fail loudly the way the too-big case just above already does. That decision is the reason this belongs in its own change rather than in a cleanup that is meant to be free of behavior changes.

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.