misc: cleanup - const-ness and more use of abseil string utilities (envoyproxy#9596)

Signed-off-by: Derek Argueta <[email protected]>

Author: derekargueta

include/envoy/network/connection_handler.h

@@ -22,7 +22,7 @@ class ConnectionHandler {
   /**
    * @return uint64_t the number of active connections owned by the handler.
    */
-  virtual uint64_t numConnections() PURE;
+  virtual uint64_t numConnections() const PURE;
 
   /**
    * Increment the return value of numConnections() by one.
@@ -77,7 +77,7 @@ class ConnectionHandler {
   /**
    * @return the stat prefix used for per-handler stats.
    */
-  virtual const std::string& statPrefix() PURE;
+  virtual const std::string& statPrefix() const PURE;
 
   /**
    * Used by ConnectionHandler to manage listeners.

include/envoy/router/route_config_update_receiver.h

@@ -53,7 +53,7 @@ class RouteConfigUpdateReceiver {
   /**
    * @return std::string& the version of RouteConfiguration.
    */
-  virtual const std::string& configVersion() PURE;
+  virtual const std::string& configVersion() const PURE;
 
   /**
    * @return uint64_t the hash value of RouteConfiguration.

include/envoy/server/configuration.h

@@ -82,12 +82,12 @@ class Admin {
   /**
    * @return const std::string& the admin access log path.
    */
-  virtual const std::string& accessLogPath() PURE;
+  virtual const std::string& accessLogPath() const PURE;
 
   /**
    * @return const std::string& profiler output path.
    */
-  virtual const std::string& profilePath() PURE;
+  virtual const std::string& profilePath() const PURE;
 
   /**
    * @return Network::Address::InstanceConstSharedPtr the server address.
@@ -115,7 +115,7 @@ class Initial {
   /**
    * @return absl::optional<std::string> the path to look for flag files.
    */
-  virtual absl::optional<std::string> flagsPath() PURE;
+  virtual absl::optional<std::string> flagsPath() const PURE;
 
   /**
    * @return const envoy::config::bootstrap::v2::LayeredRuntime& runtime

include/envoy/server/listener_manager.h

@@ -169,7 +169,7 @@ class ListenerManager {
   /**
    * @return uint64_t the total number of connections owned by all listeners across all workers.
    */
-  virtual uint64_t numConnections() PURE;
+  virtual uint64_t numConnections() const PURE;
 
   /**
    * Remove a listener by name.

include/envoy/server/worker.h

@@ -35,7 +35,7 @@ class Worker {
   /**
    * @return uint64_t the number of connections across all listeners that the worker owns.
    */
-  virtual uint64_t numConnections() PURE;
+  virtual uint64_t numConnections() const PURE;
 
   /**
    * Start the worker thread.

source/common/access_log/access_log_formatter.cc

@@ -99,7 +99,7 @@ std::string JsonFormatterImpl::format(const Http::HeaderMap& request_headers,
   const auto output_struct =
       toStruct(request_headers, response_headers, response_trailers, stream_info);
 
-  std::string log_line = MessageUtil::getJsonStringFromMessage(output_struct, false, true);
+  const std::string log_line = MessageUtil::getJsonStringFromMessage(output_struct, false, true);
   return absl::StrCat(log_line, "\n");
 }
 
@@ -115,11 +115,11 @@ ProtobufWkt::Struct JsonFormatterImpl::toStruct(const Http::HeaderMap& request_h
 
     if (providers.size() == 1) {
       const auto& provider = providers.front();
-      auto val = preserve_types_
-                     ? provider->formatValue(request_headers, response_headers, response_trailers,
-                                             stream_info)
-                     : ValueUtil::stringValue(provider->format(request_headers, response_headers,
-                                                               response_trailers, stream_info));
+      const auto val =
+          preserve_types_ ? provider->formatValue(request_headers, response_headers,
+                                                  response_trailers, stream_info)
+                          : ValueUtil::stringValue(provider->format(
+                                request_headers, response_headers, response_trailers, stream_info));
 
       (*fields)[pair.first] = val;
     } else {
@@ -144,7 +144,7 @@ void AccessLogFormatParser::parseCommandHeader(const std::string& token, const s
     throw EnvoyException(
         // Header format rules support only one alternative header.
         // docs/root/configuration/access_log.rst#format-rules
-        fmt::format("More than 1 alternative header specified in token: {}", token));
+        absl::StrCat("More than 1 alternative header specified in token: ", token));
   }
   if (subs.size() == 1) {
     alternative_header = subs.front();
@@ -163,24 +163,24 @@ void AccessLogFormatParser::parseCommand(const std::string& token, const size_t
                                          std::vector<std::string>& sub_items,
                                          absl::optional<size_t>& max_length) {
   // TODO(dnoe): Convert this to use string_view throughout.
-  size_t end_request = token.find(')', start);
+  const size_t end_request = token.find(')', start);
   sub_items.clear();
   if (end_request != token.length() - 1) {
     // Closing bracket is not found.
     if (end_request == std::string::npos) {
-      throw EnvoyException(fmt::format("Closing bracket is missing in token: {}", token));
+      throw EnvoyException(absl::StrCat("Closing bracket is missing in token: ", token));
     }
 
     // Closing bracket should be either last one or followed by ':' to denote limitation.
     if (token[end_request + 1] != ':') {
-      throw EnvoyException(fmt::format("Incorrect position of ')' in token: {}", token));
+      throw EnvoyException(absl::StrCat("Incorrect position of ')' in token: ", token));
     }
 
     const auto length_str = absl::string_view(token).substr(end_request + 2);
     uint64_t length_value;
 
     if (!absl::SimpleAtoi(length_str, &length_value)) {
-      throw EnvoyException(fmt::format("Length must be an integer, given: {}", length_str));
+      throw EnvoyException(absl::StrCat("Length must be an integer, given: ", length_str));
     }
 
     max_length = length_value;
@@ -218,7 +218,7 @@ std::vector<FormatterProviderPtr> AccessLogFormatParser::parse(const std::string
       }
 
       std::smatch m;
-      std::string search_space = format.substr(pos);
+      const std::string search_space = format.substr(pos);
       if (!(std::regex_search(search_space, m, command_w_args_regex) || m.position() == 0)) {
         throw EnvoyException(
             fmt::format("Incorrect configuration: {}. Couldn't find valid command at position {}",
@@ -228,7 +228,7 @@ std::vector<FormatterProviderPtr> AccessLogFormatParser::parse(const std::string
       const std::string match = m.str(0);
       const std::string token = match.substr(1, match.length() - 2);
       pos += 1;
-      int command_end_position = pos + token.length();
+      const int command_end_position = pos + token.length();
 
       if (absl::StartsWith(token, "REQ(")) {
         std::string main_header, alternative_header;
@@ -332,15 +332,15 @@ class StreamInfoOptionalStringFieldExtractor : public StreamInfoFormatter::Field
 
   // StreamInfoFormatter::FieldExtractor
   std::string extract(const StreamInfo::StreamInfo& stream_info) const override {
-    auto str = field_extractor_(stream_info);
+    const auto str = field_extractor_(stream_info);
     if (!str) {
       return UnspecifiedValueString;
     }
 
     return str.value();
   }
   ProtobufWkt::Value extractValue(const StreamInfo::StreamInfo& stream_info) const override {
-    auto str = field_extractor_(stream_info);
+    const auto str = field_extractor_(stream_info);
     if (!str) {
       return unspecifiedValue();
     }
@@ -362,15 +362,15 @@ class StreamInfoDurationFieldExtractor : public StreamInfoFormatter::FieldExtrac
 
   // StreamInfoFormatter::FieldExtractor
   std::string extract(const StreamInfo::StreamInfo& stream_info) const override {
-    auto millis = extractMillis(stream_info);
+    const auto millis = extractMillis(stream_info);
     if (!millis) {
       return UnspecifiedValueString;
     }
 
     return fmt::format_int(millis.value()).str();
   }
   ProtobufWkt::Value extractValue(const StreamInfo::StreamInfo& stream_info) const override {
-    auto millis = extractMillis(stream_info);
+    const auto millis = extractMillis(stream_info);
     if (!millis) {
       return unspecifiedValue();
     }
@@ -380,7 +380,7 @@ class StreamInfoDurationFieldExtractor : public StreamInfoFormatter::FieldExtrac
 
 private:
   absl::optional<uint32_t> extractMillis(const StreamInfo::StreamInfo& stream_info) const {
-    auto time = field_extractor_(stream_info);
+    const auto time = field_extractor_(stream_info);
     if (time) {
       return std::chrono::duration_cast<std::chrono::milliseconds>(time.value()).count();
     }

source/common/common/perf_annotation.cc

@@ -108,7 +108,7 @@ std::string PerfAnnotationContext::toString() {
             : std::to_string(
                   std::chrono::duration_cast<std::chrono::nanoseconds>(stats.total_).count() /
                   count));
-    columns[3].push_back(fmt::format("{}", stats.stddev_.computeStandardDeviation()));
+    columns[3].push_back(absl::StrCat("", stats.stddev_.computeStandardDeviation()));
     columns[4].push_back(nanoseconds_string(stats.min_));
     columns[5].push_back(nanoseconds_string(stats.max_));
     const CategoryDescription& category_description = p->first;

source/common/config/config_provider_impl.h

@@ -204,7 +204,7 @@ class ConfigSubscriptionCommonBase : protected Logger::Loggable<Logger::Id::conf
                                ConfigProviderManagerImplBase& config_provider_manager,
                                Server::Configuration::FactoryContext& factory_context)
       : name_(name), tls_(factory_context.threadLocal().allocateSlot()),
-        init_target_(fmt::format("ConfigSubscriptionCommonBase {}", name_), [this]() { start(); }),
+        init_target_(absl::StrCat("ConfigSubscriptionCommonBase ", name_), [this]() { start(); }),
         manager_identifier_(manager_identifier), config_provider_manager_(config_provider_manager),
         time_source_(factory_context.timeSource()),
         last_updated_(factory_context.timeSource().systemTime()) {

source/common/filesystem/inotify/watcher_impl.cc

@@ -35,7 +35,7 @@ void WatcherImpl::addWatch(const std::string& path, uint32_t events, OnChangedCb
   // and then synthetically raise per file events.
   size_t last_slash = path.rfind('/');
   if (last_slash == std::string::npos) {
-    throw EnvoyException(fmt::format("invalid watch path {}", path));
+    throw EnvoyException(absl::StrCat("invalid watch path ", path));
   }
 
   std::string directory = last_slash != 0 ? path.substr(0, last_slash) : "/";

source/common/filesystem/kqueue/watcher_impl.cc

@@ -34,7 +34,7 @@ WatcherImpl::~WatcherImpl() {
 void WatcherImpl::addWatch(const std::string& path, uint32_t events, Watcher::OnChangedCb cb) {
   FileWatchPtr watch = addWatch(path, events, cb, false);
   if (watch == nullptr) {
-    throw EnvoyException(fmt::format("invalid watch path {}", path));
+    throw EnvoyException(absl::StrCat("invalid watch path ", path));
   }
 }
 

source/common/filesystem/posix/filesystem_impl.cc

@@ -17,6 +17,7 @@
 #include "common/filesystem/filesystem_impl.h"
 
 #include "absl/strings/match.h"
+#include "absl/strings/str_cat.h"
 
 namespace Envoy {
 namespace Filesystem {
@@ -91,14 +92,14 @@ ssize_t InstanceImplPosix::fileSize(const std::string& path) {
 
 std::string InstanceImplPosix::fileReadToEnd(const std::string& path) {
   if (illegalPath(path)) {
-    throw EnvoyException(fmt::format("Invalid path: {}", path));
+    throw EnvoyException(absl::StrCat("Invalid path: ", path));
   }
 
   std::ios::sync_with_stdio(false);
 
   std::ifstream file(path);
   if (file.fail()) {
-    throw EnvoyException(fmt::format("unable to read file: {}", path));
+    throw EnvoyException(absl::StrCat("unable to read file: ", path));
   }
 
   std::stringstream file_string;

source/common/filesystem/win32/filesystem_impl.cc

@@ -84,7 +84,7 @@ ssize_t InstanceImplWin32::fileSize(const std::string& path) {
 
 std::string InstanceImplWin32::fileReadToEnd(const std::string& path) {
   if (illegalPath(path)) {
-    throw EnvoyException(fmt::format("Invalid path: {}", path));
+    throw EnvoyException(absl::StrCat("Invalid path: ", path));
   }
 
   std::ios::sync_with_stdio(false);
@@ -93,7 +93,7 @@ std::string InstanceImplWin32::fileReadToEnd(const std::string& path) {
   // 0x1a will be treated as EOF
   std::ifstream file(path, std::ios_base::binary);
   if (file.fail()) {
-    throw EnvoyException(fmt::format("unable to read file: {}", path));
+    throw EnvoyException(absl::StrCat("unable to read file: ", path));
   }
 
   std::stringstream file_string;

source/common/grpc/google_async_client_impl.cc

@@ -84,7 +84,7 @@ GoogleAsyncClientImpl::GoogleAsyncClientImpl(
   // Initialize client stats.
   stats_.streams_total_ = &scope_->counter("streams_total");
   for (uint32_t i = 0; i <= Status::WellKnownGrpcStatus::MaximumKnown; ++i) {
-    stats_.streams_closed_[i] = &scope_->counter(fmt::format("streams_closed_{}", i));
+    stats_.streams_closed_[i] = &scope_->counter(absl::StrCat("streams_closed_", i));
   }
 }
 

source/common/grpc/google_grpc_creds_impl.cc

@@ -166,8 +166,8 @@ getGoogleGrpcChannelCredentials(const envoy::config::core::v3alpha::GrpcService&
         google_grpc_credentials_factory_name);
   }
   if (credentials_factory == nullptr) {
-    throw EnvoyException(fmt::format("Unknown google grpc credentials factory: {}",
-                                     google_grpc_credentials_factory_name));
+    throw EnvoyException(absl::StrCat("Unknown google grpc credentials factory: ",
+                                      google_grpc_credentials_factory_name));
   }
   return credentials_factory->getChannelCredentials(grpc_service, api);
 }

source/common/http/conn_manager_config.h

@@ -209,7 +209,7 @@ class ConnectionManagerConfig {
    * @return the time in milliseconds the connection manager will wait between issuing a "shutdown
    *         notice" to the time it will issue a full GOAWAY and not accept any new streams.
    */
-  virtual std::chrono::milliseconds drainTimeout() PURE;
+  virtual std::chrono::milliseconds drainTimeout() const PURE;
 
   /**
    * @return FilterChainFactory& the HTTP level filter factory to build the connection's filter
@@ -221,7 +221,7 @@ class ConnectionManagerConfig {
    * @return whether the connection manager will generate a fresh x-request-id if the request does
    *         not have one.
    */
-  virtual bool generateRequestId() PURE;
+  virtual bool generateRequestId() const PURE;
 
   /**
    * @return whether the x-request-id should not be reset on edge entry inside mesh
@@ -289,12 +289,13 @@ class ConnectionManagerConfig {
   /**
    * @return const std::string& the server name to write into responses.
    */
-  virtual const std::string& serverName() PURE;
+  virtual const std::string& serverName() const PURE;
 
   /**
    * @return ServerHeaderTransformation the transformation to apply to Server response headers.
    */
-  virtual HttpConnectionManagerProto::ServerHeaderTransformation serverHeaderTransformation() PURE;
+  virtual HttpConnectionManagerProto::ServerHeaderTransformation
+  serverHeaderTransformation() const PURE;
 
   /**
    * @return ConnectionManagerStats& the stats to write to.
@@ -310,7 +311,7 @@ class ConnectionManagerConfig {
    * @return bool whether to use the remote address for populating XFF, determining internal request
    *         status, etc. or to assume that XFF will already be populated with the remote address.
    */
-  virtual bool useRemoteAddress() PURE;
+  virtual bool useRemoteAddress() const PURE;
 
   /**
    * @return InternalAddressConfig configuration for user defined internal addresses.
@@ -339,7 +340,7 @@ class ConnectionManagerConfig {
   /**
    * @return ForwardClientCertType the configuration of how to forward the client cert information.
    */
-  virtual ForwardClientCertType forwardClientCert() PURE;
+  virtual ForwardClientCertType forwardClientCert() const PURE;
 
   /**
    * @return vector of ClientCertDetailsType the configuration of the current client cert's details

source/common/http/conn_manager_impl.cc

@@ -42,6 +42,7 @@
 
 #include "absl/strings/escaping.h"
 #include "absl/strings/match.h"
+#include "absl/strings/str_cat.h"
 
 namespace Envoy {
 namespace Http {
@@ -483,7 +484,7 @@ void ConnectionManagerImpl::chargeTracingStats(const Tracing::Reason& tracing_re
     break;
   default:
     throw std::invalid_argument(
-        fmt::format("invalid tracing reason, value: {}", static_cast<int32_t>(tracing_reason)));
+        absl::StrCat("invalid tracing reason, value: ", static_cast<int32_t>(tracing_reason)));
   }
 }
 

source/common/http/hash_policy.cc

@@ -4,6 +4,8 @@
 
 #include "common/http/utility.h"
 
+#include "absl/strings/str_cat.h"
+
 namespace Envoy {
 namespace Http {
 
@@ -143,7 +145,7 @@ HashPolicyImpl::HashPolicyImpl(
       break;
     default:
       throw EnvoyException(
-          fmt::format("Unsupported hash policy {}", hash_policy->policy_specifier_case()));
+          absl::StrCat("Unsupported hash policy ", hash_policy->policy_specifier_case()));
     }
   }
 }

source/common/http/http2/codec_impl.cc

@@ -825,7 +825,7 @@ void ConnectionImpl::sendPendingFrames() {
     return;
   }
 
-  int rc = nghttp2_session_send(session_);
+  const int rc = nghttp2_session_send(session_);
   if (rc != 0) {
     ASSERT(rc == NGHTTP2_ERR_CALLBACK_FAILURE);
     // For errors caused by the pending outbound frame flood the FrameFloodException has
@@ -837,7 +837,7 @@ void ConnectionImpl::sendPendingFrames() {
       throw FrameFloodException("Too many frames in the outbound queue.");
     }
 
-    throw CodecProtocolException(fmt::format("{}", nghttp2_strerror(rc)));
+    throw CodecProtocolException(std::string(nghttp2_strerror(rc)));
   }
 
   // See ConnectionImpl::StreamImpl::resetStream() for why we do this. This is an uncommon event,