clang-tidy: validate casing of private members (envoyproxy#8503)

Description: standardize private class members as `lower_case_`. Risk Level: low Testing: existing Docs Changes: N/A Release Notes: N/A Signed-off-by: Derek Argueta <[email protected]>
KNWBIN · Oct 9, 2019 · 10f3f9e · 10f3f9e
1 parent 7348e44
commit 10f3f9e
Show file tree

Hide file tree

Showing 15 changed files with 79 additions and 73 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -68,6 +68,12 @@ CheckOptions:
 
   - key:             readability-identifier-naming.ParameterCase
     value:           'lower_case'
-
+
+  - key:             readability-identifier-naming.PrivateMemberCase
+    value:           'lower_case'
+
+  - key:             readability-identifier-naming.PrivateMemberSuffix
+    value:           '_'
+
   - key:             readability-identifier-naming.TypeAliasCase
     value:           'CamelCase'
diff --git a/source/common/config/grpc_stream.h b/source/common/config/grpc_stream.h
@@ -35,8 +35,12 @@ class GrpcStream : public Grpc::AsyncStreamCallbacks<ResponseProto>,
           rate_limit_settings.max_tokens_, time_source_, rate_limit_settings.fill_rate_);
       drain_request_timer_ = dispatcher.createTimer([this]() { callbacks_->onWriteable(); });
     }
-    backoff_strategy_ = std::make_unique<JitteredBackOffStrategy>(RETRY_INITIAL_DELAY_MS,
-                                                                  RETRY_MAX_DELAY_MS, random_);
+
+    // TODO(htuch): Make this configurable.
+    static constexpr uint32_t RetryInitialDelayMs = 500;
+    static constexpr uint32_t RetryMaxDelayMs = 30000; // Do not cross more than 30s
+    backoff_strategy_ =
+        std::make_unique<JitteredBackOffStrategy>(RetryInitialDelayMs, RetryMaxDelayMs, random_);
   }
 
   void establishNewStream() {
@@ -128,10 +132,6 @@ class GrpcStream : public Grpc::AsyncStreamCallbacks<ResponseProto>,
 
   GrpcStreamCallbacks<ResponseProto>* const callbacks_;
 
-  // TODO(htuch): Make this configurable or some static.
-  const uint32_t RETRY_INITIAL_DELAY_MS = 500;
-  const uint32_t RETRY_MAX_DELAY_MS = 30000; // Do not cross more than 30s
-
   Grpc::AsyncClient<RequestProto, ResponseProto> async_client_;
   Grpc::AsyncStream<RequestProto> stream_{};
   const Protobuf::MethodDescriptor& service_method_;

diff --git a/source/common/http/codes.h b/source/common/http/codes.h
@@ -78,7 +78,6 @@ class CodeStatsImpl : public CodeStats {
   const Stats::StatName upstream_rq_5xx_;
   const Stats::StatName upstream_rq_unknown_;
   const Stats::StatName upstream_rq_completed_;
-  const Stats::StatName upstream_rq_time;
   const Stats::StatName upstream_rq_time_;
   const Stats::StatName vcluster_;
   const Stats::StatName vhost_;

diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc
@@ -377,6 +377,9 @@ DiskLayer::DiskLayer(absl::string_view name, const std::string& path, Api::Api&
 
 void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix, uint32_t depth,
                               Api::Api& api) {
+  // Maximum recursion depth for walkDirectory().
+  static constexpr uint32_t MaxWalkDepth = 16;
+
   ENVOY_LOG(debug, "walking directory: {}", path);
   if (depth > MaxWalkDepth) {
     throw EnvoyException(fmt::format("Walk recursion depth exceeded {}", MaxWalkDepth));

diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h
@@ -181,8 +181,6 @@ class DiskLayer : public OverrideLayerImpl, Logger::Loggable<Logger::Id::runtime
                      Api::Api& api);
 
   const std::string path_;
-  // Maximum recursion depth for walkDirectory().
-  const uint32_t MaxWalkDepth = 16;
   const Filesystem::WatcherPtr watcher_;
 };
 

diff --git a/source/common/upstream/health_discovery_service.cc b/source/common/upstream/health_discovery_service.cc
@@ -7,6 +7,16 @@
 namespace Envoy {
 namespace Upstream {
 
+/**
+ * TODO(lilika): Add API knob for RetryInitialDelayMilliseconds
+ * and RetryMaxDelayMilliseconds, instead of hardcoding them.
+ *
+ * Parameters of the jittered backoff strategy that defines how often
+ * we retry to establish a stream to the management server
+ */
+static constexpr uint32_t RetryInitialDelayMilliseconds = 1000;
+static constexpr uint32_t RetryMaxDelayMilliseconds = 30000;
+
 HdsDelegate::HdsDelegate(Stats::Scope& scope, Grpc::RawAsyncClientPtr async_client,
                          Event::Dispatcher& dispatcher, Runtime::Loader& runtime,
                          Envoy::Stats::Store& stats, Ssl::ContextManager& ssl_context_manager,
@@ -19,7 +29,7 @@ HdsDelegate::HdsDelegate(Stats::Scope& scope, Grpc::RawAsyncClientPtr async_clie
       service_method_(*Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
           "envoy.service.discovery.v2.HealthDiscoveryService.StreamHealthCheck")),
       async_client_(std::move(async_client)), dispatcher_(dispatcher), runtime_(runtime),
-      store_stats(stats), ssl_context_manager_(ssl_context_manager), random_(random),
+      store_stats_(stats), ssl_context_manager_(ssl_context_manager), random_(random),
       info_factory_(info_factory), access_log_manager_(access_log_manager), cm_(cm),
       local_info_(local_info), admin_(admin), singleton_manager_(singleton_manager), tls_(tls),
       validation_visitor_(validation_visitor), api_(api) {
@@ -148,7 +158,7 @@ void HdsDelegate::processMessage(
 
     // Create HdsCluster
     hds_clusters_.emplace_back(new HdsCluster(admin_, runtime_, cluster_config, bind_config,
-                                              store_stats, ssl_context_manager_, false,
+                                              store_stats_, ssl_context_manager_, false,
                                               info_factory_, cm_, local_info_, dispatcher_, random_,
                                               singleton_manager_, tls_, validation_visitor_, api_));
 

diff --git a/source/common/upstream/health_discovery_service.h b/source/common/upstream/health_discovery_service.h
@@ -154,7 +154,7 @@ class HdsDelegate : Grpc::AsyncStreamCallbacks<envoy::service::discovery::v2::He
       stream_{};
   Event::Dispatcher& dispatcher_;
   Runtime::Loader& runtime_;
-  Envoy::Stats::Store& store_stats;
+  Envoy::Stats::Store& store_stats_;
   Ssl::ContextManager& ssl_context_manager_;
   Runtime::RandomGenerator& random_;
   ClusterInfoFactory& info_factory_;
@@ -175,16 +175,6 @@ class HdsDelegate : Grpc::AsyncStreamCallbacks<envoy::service::discovery::v2::He
   Event::TimerPtr hds_retry_timer_;
   BackOffStrategyPtr backoff_strategy_;
 
-  /**
-   * TODO(lilika): Add API knob for RetryInitialDelayMilliseconds
-   * and RetryMaxDelayMilliseconds, instead of hardcoding them.
-   *
-   * Parameters of the jittered backoff strategy that defines how often
-   * we retry to establish a stream to the management server
-   */
-  const uint32_t RetryInitialDelayMilliseconds = 1000;
-  const uint32_t RetryMaxDelayMilliseconds = 30000;
-
   // Soft limit on size of the cluster’s connections read and write buffers.
   static constexpr uint32_t ClusterConnectionBufferLimitBytes = 32768;
 

diff --git a/source/common/upstream/outlier_detection_impl.cc b/source/common/upstream/outlier_detection_impl.cc
@@ -37,8 +37,8 @@ DetectorHostMonitorImpl::DetectorHostMonitorImpl(std::shared_ptr<DetectorImpl> d
                                                  HostSharedPtr host)
     : detector_(detector), host_(host),
       // add Success Rate monitors
-      external_origin_SR_monitor_(envoy::data::cluster::v2alpha::OutlierEjectionType::SUCCESS_RATE),
-      local_origin_SR_monitor_(
+      external_origin_sr_monitor_(envoy::data::cluster::v2alpha::OutlierEjectionType::SUCCESS_RATE),
+      local_origin_sr_monitor_(
           envoy::data::cluster::v2alpha::OutlierEjectionType::SUCCESS_RATE_LOCAL_ORIGIN) {
   // Setup method to call when putResult is invoked. Depending on the config's
   // split_external_local_origin_errors_ boolean value different method is called.
@@ -59,12 +59,12 @@ void DetectorHostMonitorImpl::uneject(MonotonicTime unejection_time) {
 }
 
 void DetectorHostMonitorImpl::updateCurrentSuccessRateBucket() {
-  external_origin_SR_monitor_.updateCurrentSuccessRateBucket();
-  local_origin_SR_monitor_.updateCurrentSuccessRateBucket();
+  external_origin_sr_monitor_.updateCurrentSuccessRateBucket();
+  local_origin_sr_monitor_.updateCurrentSuccessRateBucket();
 }
 
 void DetectorHostMonitorImpl::putHttpResponseCode(uint64_t response_code) {
-  external_origin_SR_monitor_.incTotalReqCounter();
+  external_origin_sr_monitor_.incTotalReqCounter();
   if (Http::CodeUtility::is5xx(response_code)) {
     std::shared_ptr<DetectorImpl> detector = detector_.lock();
     if (!detector) {
@@ -87,7 +87,7 @@ void DetectorHostMonitorImpl::putHttpResponseCode(uint64_t response_code) {
       detector->onConsecutive5xx(host_.lock());
     }
   } else {
-    external_origin_SR_monitor_.incSuccessReqCounter();
+    external_origin_sr_monitor_.incSuccessReqCounter();
     consecutive_5xx_ = 0;
     consecutive_gateway_failure_ = 0;
   }
@@ -184,7 +184,7 @@ void DetectorHostMonitorImpl::localOriginFailure() {
     // It's possible for the cluster/detector to go away while we still have a host in use.
     return;
   }
-  local_origin_SR_monitor_.incTotalReqCounter();
+  local_origin_sr_monitor_.incTotalReqCounter();
   if (++consecutive_local_origin_failure_ ==
       detector->runtime().snapshot().getInteger(
           "outlier_detection.consecutive_local_origin_failure",
@@ -200,8 +200,8 @@ void DetectorHostMonitorImpl::localOriginNoFailure() {
     return;
   }
 
-  local_origin_SR_monitor_.incTotalReqCounter();
-  local_origin_SR_monitor_.incSuccessReqCounter();
+  local_origin_sr_monitor_.incTotalReqCounter();
+  local_origin_sr_monitor_.incSuccessReqCounter();
 
   resetConsecutiveLocalOriginFailure();
 }
@@ -260,8 +260,8 @@ DetectorImpl::DetectorImpl(const Cluster& cluster,
       interval_timer_(dispatcher.createTimer([this]() -> void { onIntervalTimer(); })),
       event_logger_(event_logger) {
   // Insert success rate initial numbers for each type of SR detector
-  external_origin_SR_num_ = {-1, -1};
-  local_origin_SR_num_ = {-1, -1};
+  external_origin_sr_num_ = {-1, -1};
+  local_origin_sr_num_ = {-1, -1};
 }
 
 DetectorImpl::~DetectorImpl() {

diff --git a/source/common/upstream/outlier_detection_impl.h b/source/common/upstream/outlier_detection_impl.h
@@ -156,8 +156,8 @@ class DetectorHostMonitorImpl : public DetectorHostMonitor {
   }
 
   const SuccessRateMonitor& getSRMonitor(SuccessRateMonitorType type) const {
-    return (SuccessRateMonitorType::ExternalOrigin == type) ? external_origin_SR_monitor_
-                                                            : local_origin_SR_monitor_;
+    return (SuccessRateMonitorType::ExternalOrigin == type) ? external_origin_sr_monitor_
+                                                            : local_origin_sr_monitor_;
   }
 
   SuccessRateMonitor& getSRMonitor(SuccessRateMonitorType type) {
@@ -197,8 +197,8 @@ class DetectorHostMonitorImpl : public DetectorHostMonitor {
   //   and for external origin failures when external/local events are split
   // - local origin: for local events when external/local events are split and
   //   not used when external/local events are not split.
-  SuccessRateMonitor external_origin_SR_monitor_;
-  SuccessRateMonitor local_origin_SR_monitor_;
+  SuccessRateMonitor external_origin_sr_monitor_;
+  SuccessRateMonitor local_origin_sr_monitor_;
 
   void putResultNoLocalExternalSplit(Result result, absl::optional<uint64_t> code);
   void putResultWithLocalExternalSplit(Result result, absl::optional<uint64_t> code);
@@ -396,17 +396,17 @@ class DetectorImpl : public Detector, public std::enable_shared_from_this<Detect
   EventLoggerSharedPtr event_logger_;
 
   // EjectionPair for external and local origin events.
-  // When external/local origin events are not split, external_origin_SR_num_ are used for
-  // both types of events: external and local. local_origin_SR_num_ is not used.
-  // When external/local origin events are split, external_origin_SR_num_ are used only
-  // for external events and local_origin_SR_num_ is used for local origin events.
-  EjectionPair external_origin_SR_num_;
-  EjectionPair local_origin_SR_num_;
+  // When external/local origin events are not split, external_origin_sr_num_ are used for
+  // both types of events: external and local. local_origin_sr_num_ is not used.
+  // When external/local origin events are split, external_origin_sr_num_ are used only
+  // for external events and local_origin_sr_num_ is used for local origin events.
+  EjectionPair external_origin_sr_num_;
+  EjectionPair local_origin_sr_num_;
 
   const EjectionPair& getSRNums(DetectorHostMonitor::SuccessRateMonitorType monitor_type) const {
     return (DetectorHostMonitor::SuccessRateMonitorType::ExternalOrigin == monitor_type)
-               ? external_origin_SR_num_
-               : local_origin_SR_num_;
+               ? external_origin_sr_num_
+               : local_origin_sr_num_;
   }
   EjectionPair& getSRNums(DetectorHostMonitor::SuccessRateMonitorType monitor_type) {
     return const_cast<EjectionPair&>(

diff --git a/source/exe/main_common.h b/source/exe/main_common.h
@@ -122,8 +122,8 @@ class MainCommon {
 
 private:
 #ifdef ENVOY_HANDLE_SIGNALS
-  Envoy::SignalAction handle_sigs;
-  Envoy::TerminateHandler log_on_terminate;
+  Envoy::SignalAction handle_sigs_;
+  Envoy::TerminateHandler log_on_terminate_;
 #endif
 
   PlatformImpl platform_impl_;

diff --git a/source/server/hot_restarting_base.cc b/source/server/hot_restarting_base.cc
@@ -8,6 +8,8 @@ namespace Server {
 
 using HotRestartMessage = envoy::HotRestartMessage;
 
+static constexpr uint64_t MaxSendmsgSize = 4096;
+
 void HotRestartingBase::initDomainSocketAddress(sockaddr_un* address) {
   memset(address, 0, sizeof(*address));
   address->sun_family = AF_UNIX;
@@ -16,8 +18,8 @@ void HotRestartingBase::initDomainSocketAddress(sockaddr_un* address) {
 sockaddr_un HotRestartingBase::createDomainSocketAddress(uint64_t id, const std::string& role) {
   // Right now we only allow a maximum of 3 concurrent envoy processes to be running. When the third
   // starts up it will kill the oldest parent.
-  const uint64_t MAX_CONCURRENT_PROCESSES = 3;
-  id = id % MAX_CONCURRENT_PROCESSES;
+  static constexpr uint64_t MaxConcurrentProcesses = 3;
+  id = id % MaxConcurrentProcesses;
 
   // This creates an anonymous domain socket name (where the first byte of the name of \0).
   sockaddr_un address;
@@ -122,8 +124,8 @@ void HotRestartingBase::getPassedFdIfPresent(HotRestartMessage* out, msghdr* mes
   }
 }
 
-// While in use, recv_buf_ is always >= MaxSendmsgSize. In between messages, it is kept empty, to be
-// grown back to MaxSendmsgSize at the start of the next message.
+// While in use, recv_buf_ is always >= MaxSendmsgSize. In between messages, it is kept empty,
+// to be grown back to MaxSendmsgSize at the start of the next message.
 void HotRestartingBase::initRecvBufIfNewMessage() {
   if (recv_buf_.empty()) {
     ASSERT(cur_msg_recvd_bytes_ == 0);

diff --git a/source/server/hot_restarting_base.h b/source/server/hot_restarting_base.h
@@ -61,16 +61,14 @@ class HotRestartingBase {
   std::unique_ptr<envoy::HotRestartMessage> parseProtoAndResetState();
   void initRecvBufIfNewMessage();
 
-  // An int in [0, MAX_CONCURRENT_PROCESSES). As hot restarts happen, each next process gets the
+  // An int in [0, MaxConcurrentProcesses). As hot restarts happen, each next process gets the
   // next of 0,1,2,0,1,...
   // A HotRestartingBase's domain socket's name contains its base_id_ value, and so we can use
   // this value to determine which domain socket name to treat as our parent, and which to treat as
   // our child. (E.g. if we are 2, 1 is parent and 0 is child).
   const uint64_t base_id_;
   int my_domain_socket_{-1};
 
-  const uint64_t MaxSendmsgSize = 4096;
-
   // State for the receiving half of the protocol.
   //
   // When filled, the size in bytes that the in-flight HotRestartMessage should be.

diff --git a/test/common/network/dns_impl_test.cc b/test/common/network/dns_impl_test.cc
@@ -55,7 +55,7 @@ class TestDnsServerQuery {
 public:
   TestDnsServerQuery(ConnectionPtr connection, const HostMap& hosts_a, const HostMap& hosts_aaaa,
                      const CNameMap& cnames, const std::chrono::seconds& record_ttl)
-      : connection_(std::move(connection)), hosts_A_(hosts_a), hosts_AAAA_(hosts_aaaa),
+      : connection_(std::move(connection)), hosts_a_(hosts_a), hosts_aaaa_(hosts_aaaa),
         cnames_(cnames), record_ttl_(record_ttl) {
     connection_->addReadFilter(Network::ReadFilterSharedPtr{new ReadFilter(*this)});
   }
@@ -119,8 +119,8 @@ class TestDnsServerQuery {
         long name_len;
         // Get host name from query and use the name to lookup a record
         // in a host map. If the query type is of type A, then perform the lookup in
-        // the hosts_A_ host map. If the query type is of type AAAA, then perform the
-        // lookup in the hosts_AAAA_ host map.
+        // the hosts_a_ host map. If the query type is of type AAAA, then perform the
+        // lookup in the hosts_aaaa_ host map.
         char* name;
         ASSERT_EQ(ARES_SUCCESS, ares_expand_name(question, request, size_, &name, &name_len));
         const std::list<std::string>* ips = nullptr;
@@ -147,13 +147,13 @@ class TestDnsServerQuery {
         }
         ASSERT_TRUE(q_type == T_A || q_type == T_AAAA);
         if (q_type == T_A) {
-          auto it = parent_.hosts_A_.find(hostLookup);
-          if (it != parent_.hosts_A_.end()) {
+          auto it = parent_.hosts_a_.find(hostLookup);
+          if (it != parent_.hosts_a_.end()) {
             ips = &it->second;
           }
         } else {
-          auto it = parent_.hosts_AAAA_.find(hostLookup);
-          if (it != parent_.hosts_AAAA_.end()) {
+          auto it = parent_.hosts_aaaa_.find(hostLookup);
+          if (it != parent_.hosts_aaaa_.end()) {
             ips = &it->second;
           }
         }
@@ -248,8 +248,8 @@ class TestDnsServerQuery {
 
 private:
   ConnectionPtr connection_;
-  const HostMap& hosts_A_;
-  const HostMap& hosts_AAAA_;
+  const HostMap& hosts_a_;
+  const HostMap& hosts_aaaa_;
   const CNameMap& cnames_;
   const std::chrono::seconds& record_ttl_;
 };
@@ -265,16 +265,16 @@ class TestDnsServer : public ListenerCallbacks {
   }
 
   void onNewConnection(ConnectionPtr&& new_connection) override {
-    TestDnsServerQuery* query = new TestDnsServerQuery(std::move(new_connection), hosts_A_,
-                                                       hosts_AAAA_, cnames_, record_ttl_);
+    TestDnsServerQuery* query = new TestDnsServerQuery(std::move(new_connection), hosts_a_,
+                                                       hosts_aaaa_, cnames_, record_ttl_);
     queries_.emplace_back(query);
   }
 
   void addHosts(const std::string& hostname, const IpList& ip, const RecordType& type) {
     if (type == RecordType::A) {
-      hosts_A_[hostname] = ip;
+      hosts_a_[hostname] = ip;
     } else if (type == RecordType::AAAA) {
-      hosts_AAAA_[hostname] = ip;
+      hosts_aaaa_[hostname] = ip;
     }
   }
 
@@ -287,8 +287,8 @@ class TestDnsServer : public ListenerCallbacks {
 private:
   Event::Dispatcher& dispatcher_;
 
-  HostMap hosts_A_;
-  HostMap hosts_AAAA_;
+  HostMap hosts_a_;
+  HostMap hosts_aaaa_;
   CNameMap cnames_;
   std::chrono::seconds record_ttl_;
   // All queries are tracked so we can do resource reclamation when the test is