cmcfarlen commented on code in PR #11668:
URL: https://github.com/apache/trafficserver/pull/11668#discussion_r1716152988
##########
plugins/experimental/stek_share/log_store.cc:
##########
@@ -90,7 +91,7 @@ STEKShareLogStore::write_at(uint64_t index,
nuraft::ptr<nuraft::log_entry> &entr
while (itr != logs_.end()) {
itr = logs_.erase(itr);
}
- logs_[index] = clone;
+ logs_[index] = std::move(clone);
Review Comment:
This can be `logs_.emplace(index, make_clone(entry))` to get `std::forward`
##########
src/iocore/net/SSLSessionCache.cc:
##########
@@ -353,7 +354,7 @@ SSLOriginSessionCache::insert_session(const std::string
&lookup_key, SSL_SESSION
// Create the shared pointer to the session, with the custom deleter
std::shared_ptr<SSL_SESSION> shared_sess(sess_ptr, SSLSessDeleter);
ssl_curve_id curve = (ssl == nullptr) ? 0 :
SSLGetCurveNID(ssl);
- std::unique_ptr<SSLOriginSession> ssl_orig_session(new
SSLOriginSession(lookup_key, curve, shared_sess));
+ std::unique_ptr<SSLOriginSession> ssl_orig_session(new
SSLOriginSession(lookup_key, curve, std::move(shared_sess)));
Review Comment:
You can avoid the temp and move if you move the construction to the argument:
```
std::unique_ptr<SSLOriginSession> ssl_orig_session(new
SSLOriginSession(lookup_key, curve, std::shared_ptr<SSL_SESSION>(sess_ptr,
SSLSessDeleter)));
```
##########
src/traffic_layout/engine.cc:
##########
@@ -362,7 +363,7 @@ LayoutEngine::remove_runroot()
return;
}
- std::string clean_root = path;
+ std::string clean_root = std::move(path);
Review Comment:
Not sure what the intent here was. Does this code want a copy of path? If
so, your std::move is counter to that goal. If not, then this is an
unnecessary temp.
##########
plugins/statichit/statichit.cc:
##########
@@ -71,7 +73,7 @@ struct StaticHitConfig {
base_path = std::filesystem::weakly_canonical(base_path);
if (std::filesystem::is_directory(base_path)) {
- dirPath = base_path;
+ dirPath = std::move(base_path);
Review Comment:
does coverity not have a problem with the assignment to `filePath` below?
##########
src/traffic_layout/engine.cc:
##########
@@ -276,7 +277,7 @@ LayoutEngine::create_runroot()
std::string value = it.second.as<std::string>();
auto iter = new_map.find(key);
if (iter != new_map.end()) {
- iter->second = value;
+ iter->second = std::move(value);
Review Comment:
You can just remove the `value` variable as its only temp
##########
src/iocore/net/SSLUtils.cc:
##########
@@ -1677,7 +1679,8 @@ SSLMultiCertConfigLoader::_store_ssl_ctx(SSLCertLookup
*lookup, const shared_SSL
std::vector<SSLLoadingContext> ctxs = this->init_server_ssl_ctx(data,
sslMultCertSettings.get());
for (const auto &loadingctx : ctxs) {
shared_SSL_CTX ctx(loadingctx.ctx, SSL_CTX_free);
- if (!sslMultCertSettings || !this->_store_single_ssl_ctx(lookup,
sslMultCertSettings, ctx, loadingctx.ctx_type, common_names)) {
+ if (!sslMultCertSettings ||
+ !this->_store_single_ssl_ctx(lookup, sslMultCertSettings,
std::move(ctx), loadingctx.ctx_type, common_names)) {
Review Comment:
Could move the construction to the arglist here as well. This will also
avoid an unused tmp if the disjunction shorts
--
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]
