You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "JosiahWI (via GitHub)" <gi...@apache.org> on 2026/08/01 10:36:51 UTC
[PR] Fix some copy instead of move Coverity CIDs [trafficserver]
JosiahWI opened a new pull request, #13470:
URL: https://github.com/apache/trafficserver/pull/13470
CIDs fixed in this patch:
1664317
1664316
1664286
1664284
1663965
1660036
1658876
1658839
1658837
1658799
1654583
1654213
1654211
1654210
1654207
1654206
1645817
1645816
1645814
1645813
1645811
1645809
1645808
1645806
1645804
1645802
1645801
1645799
1645798
1645796
1645795
1644303
1644300
1644293
1644255
1644242
1644227
1644207
--
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
Re: [PR] Fix some copy instead of move Coverity CIDs [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13470:
URL: https://github.com/apache/trafficserver/pull/13470#discussion_r3695451998
##########
src/config/ssl_multicert.cc:
##########
@@ -281,7 +281,7 @@ SSLMultiCertParser::parse_yaml(std::string_view content)
return {result, swoc::Errata("YAML parse error: {}", ex.what())};
}
- return {result, std::move(errata)};
+ return {std::move(result), std::move(errata)};
Review Comment:
This file now uses std::move in return statements, but does not include <utility>, which is the standard header that declares std::move. Relying on transitive includes can break builds across standard library implementations or future refactors; add an explicit <utility> include in this translation unit.
--
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
Re: [PR] Fix some copy instead of move Coverity CIDs [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI merged PR #13470:
URL: https://github.com/apache/trafficserver/pull/13470
--
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
Re: [PR] Fix some copy instead of move Coverity CIDs [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13470:
URL: https://github.com/apache/trafficserver/pull/13470#discussion_r3695443621
##########
include/mgmt/config/ConfigReloadTrace.h:
##########
@@ -207,7 +207,7 @@ class ConfigReloadTask : public std::enable_shared_from_this<ConfigReloadTask>
using self_type = ConfigReloadTask;
ConfigReloadTask() = default;
ConfigReloadTask(std::string_view token, std::string_view description, bool main_task, ConfigReloadTaskPtr parent)
- : _info(State::CREATED, token, description, main_task), _parent{parent}
+ : _info(State::CREATED, token, description, main_task), _parent{std::move(parent)}
{
Review Comment:
`std::move` is used in this header but the header does not include `<utility>`. Relying on transitive includes can break builds depending on include order / standard library implementation; headers should include what they use.
--
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
Re: [PR] Fix some copy instead of move Coverity CIDs [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #13470:
URL: https://github.com/apache/trafficserver/pull/13470#issuecomment-5181035904
Cherry-picked to the 10.2.x branch as ebad97a1e37d094211e171a90c18cf7bb901f5be for the 10.2.0 release. This is a partial pick; see the commit message for the omitted hunks.
--
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
Re: [PR] Fix some copy instead of move Coverity CIDs [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #13470:
URL: https://github.com/apache/trafficserver/pull/13470#issuecomment-5152020471
Copilot's comment about moving the ternary operand blocking NRVO/copy elision is possibly the most interesting dubious comment I've seen from Copilot to date. It has low confidence, which is good, because it should. This is a very subtle trap in C++: the original ternary expression has an l-value operand and an r-value operand, so the ternary expression is actually forced to cause a temporary copy of `metric_name` if that branch is taken. Coverity is right - this is a special edge case where the `std::move` is actually appropriate.
--
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