From c548853c694167dd0dec8a4900428870e5e7a8fe Mon Sep 17 00:00:00 2001 From: Ryan Northey Date: Mon, 18 Sep 2023 18:26:20 +0100 Subject: [PATCH 1/4] clang/tidy: Fixes Signed-off-by: Ryan Northey --- .clang-tidy | 39 ++++++- .../fetch_record_converter_unit_test.cc | 2 +- envoy/common/optref.h | 4 +- source/common/grpc/async_client_impl.cc | 1 + source/common/grpc/google_grpc_utils.cc | 2 +- .../network/connection_balancer_impl.cc | 2 +- .../quic/envoy_quic_server_connection.h | 3 + .../rds/rds_route_config_subscription.cc | 2 +- source/common/stats/symbol_table.h | 2 +- source/common/stats/thread_local_store.cc | 2 +- source/common/upstream/load_balancer_impl.cc | 6 +- source/extensions/clusters/eds/eds.cc | 2 +- source/server/hot_restarting_base.cc | 1 + source/server/options_impl.cc | 1 + test/benchmark/main.cc | 4 +- .../access_log_manager_impl_test.cc | 6 ++ test/common/common/utility_speed_test.cc | 100 +++++++++--------- .../grpc/async_client_manager_benchmark.cc | 4 +- test/common/http/codec_client_test.cc | 2 +- test/common/http/codes_speed_test.cc | 4 +- test/common/http/http1/codec_impl_test.cc | 6 +- test/common/http/http2/codec_impl_test.cc | 3 + test/common/http/http3/conn_pool_test.cc | 2 + .../common/network/address_impl_speed_test.cc | 4 +- test/common/network/lc_trie_speed_test.cc | 13 +-- .../quic/envoy_quic_proof_verifier_test.cc | 1 + test/common/stats/symbol_table_impl_test.cc | 10 +- test/common/stats/symbol_table_speed_test.cc | 1 + .../upstream/cluster_manager_impl_test.cc | 4 + .../default_local_address_selector_test.cc | 2 +- .../common/wasm/test_data/test_context_cpp.cc | 2 +- .../ext_authz/ext_authz_integration_test.cc | 3 +- .../filters/http/wasm/test_data/test_cpp.cc | 22 ++-- .../redis_proxy/command_lookup_speed_test.cc | 6 +- .../redis_proxy/command_split_speed_test.cc | 24 ++--- ...erministic_connection_id_generator_test.cc | 5 +- test/extensions/tracers/datadog/span_test.cc | 5 +- .../transport_sockets/tls/ssl_socket_test.cc | 12 +-- .../upstream_http_filter_integration_test.cc | 6 +- test/mocks/buffer/mocks.cc | 1 + test/tools/router_check/router.cc | 1 + test/tools/schema_validator/validator.cc | 3 +- tools/bootstrap2pb.cc | 1 + tools/spelling/spelling_dictionary.txt | 2 + 44 files changed, 199 insertions(+), 129 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 7e9d197c9921..d86ad55cb536 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -56,9 +56,36 @@ CheckOptions: value: 'CamelCase' # Ignore GoogleTest function macros. - key: readability-identifier-naming.FunctionIgnoredRegexp - value: '(TEST|TEST_F|TEST_P|INSTANTIATE_TEST_SUITE_P|MOCK_METHOD|TYPED_TEST)' + # To have the regex chomped correctly fence all items with `|` (other than first/last) + value: >- + (^AbslHashValue$| + |^called_count$| + |^case_sensitive$| + |^Create$| + |^envoy_resolve_dns$| + |^evconnlistener_free$| + |^event_base_free$| + |^(get|set)EVP_PKEY$| + |^Ip6(ntohl|htonl)$| + |^get_$| + |^HeaderHasValue(Ref)?$| + |^HeaderValueOf$| + |^Is(Superset|Subset)OfHeaders$| + |^LLVMFuzzerInitialize$| + |^LLVMFuzzerTestOneInput$| + |^MOCK_METHOD$| + |^PrepareCall$| + |^PrintTo$| + |^resolve_dns$| + |^result_type$| + |Returns(Default)?WorkerId$| + |^shutdownThread_$| + |TEST| + |^use_count$) - key: readability-identifier-naming.ParameterCase value: 'lower_case' +- key: readability-identifier-naming.ParameterIgnoredRegexp + value: (^cname_ttl_$) - key: readability-identifier-naming.PrivateMemberCase value: 'lower_case' - key: readability-identifier-naming.PrivateMemberSuffix @@ -67,11 +94,21 @@ CheckOptions: value: 'CamelCase' - key: readability-identifier-naming.TypeAliasCase value: 'CamelCase' +- key: readability-identifier-naming.TypeAliasIgnoredRegexp + value: '(result_type)' - key: readability-identifier-naming.UnionCase value: 'CamelCase' - key: readability-identifier-naming.FunctionCase value: 'camelBack' +HeaderFilterRegex: '^./source/.*|^./contrib/.*|^./test/.*' + UseColor: true WarningsAsErrors: '*' + +## The version here is arbitrary since any change to this file will +## trigger a full run of clang-tidy against all files. +## It can be useful as it seems some header changes may not trigger the +## expected rerun. +# v0 diff --git a/contrib/kafka/filters/network/test/mesh/command_handlers/fetch_record_converter_unit_test.cc b/contrib/kafka/filters/network/test/mesh/command_handlers/fetch_record_converter_unit_test.cc index 48e98bc9bfb6..fe2cc0a40b6e 100644 --- a/contrib/kafka/filters/network/test/mesh/command_handlers/fetch_record_converter_unit_test.cc +++ b/contrib/kafka/filters/network/test/mesh/command_handlers/fetch_record_converter_unit_test.cc @@ -77,7 +77,7 @@ TEST(FetchRecordConverterImpl, shouldProcessRecords) { const auto ptr = reinterpret_cast(data->data() + record_count_offset); const uint32_t record_count = be32toh(*ptr); ASSERT_EQ(record_count, 3); -} +} // NOLINT(clang-analyzer-cplusplus.NewDeleteLeaks) // Here we check whether our manual implementation really works. // https://github.com/apache/kafka/blob/3.3.2/clients/src/main/java/org/apache/kafka/common/utils/Crc32C.java diff --git a/envoy/common/optref.h b/envoy/common/optref.h index 869720b6ae4c..2983bde1b11c 100644 --- a/envoy/common/optref.h +++ b/envoy/common/optref.h @@ -48,12 +48,12 @@ template struct OptRef { /** * Helper to convert a OptRef into a ref. Behavior if !has_value() is undefined. */ - T& ref() const { return *ptr_; } + T& ref() const { return *ptr_; } // NOLINT(clang-analyzer-core.uninitialized.UndefReturn) /** * Helper to dereference an OptRef. Behavior if !has_value() is undefined. */ - T& operator*() const { return *ptr_; } + T& operator*() const { return *ptr_; } // NOLINT(clang-analyzer-core.uninitialized.UndefReturn) /** * @return true if the object has a value. diff --git a/source/common/grpc/async_client_impl.cc b/source/common/grpc/async_client_impl.cc index 7e03eaab7b28..93e779554556 100644 --- a/source/common/grpc/async_client_impl.cc +++ b/source/common/grpc/async_client_impl.cc @@ -123,6 +123,7 @@ void AsyncStreamImpl::onHeaders(Http::ResponseHeaderMapPtr&& headers, bool end_s // TODO(mattklein123): clang-tidy is showing a use after move when passing to // onReceiveInitialMetadata() above. This looks like an actual bug that I will fix in a // follow up. + // NOLINTNEXTLINE(bugprone-use-after-move) onTrailers(Http::createHeaderMap(*headers)); return; } diff --git a/source/common/grpc/google_grpc_utils.cc b/source/common/grpc/google_grpc_utils.cc index a3510685233c..fbe891306692 100644 --- a/source/common/grpc/google_grpc_utils.cc +++ b/source/common/grpc/google_grpc_utils.cc @@ -79,7 +79,7 @@ grpc::ByteBuffer GoogleGrpcUtils::makeByteBuffer(Buffer::InstancePtr&& buffer_in slices.emplace_back(raw_slice.mem_, raw_slice.len_, &BufferInstanceContainer::derefBufferInstanceContainer, container); } - return {&slices[0], slices.size()}; + return {&slices[0], slices.size()}; // NOLINT(clang-analyzer-cplusplus.NewDeleteLeaks) } class GrpcSliceBufferFragmentImpl : public Buffer::BufferFragment { diff --git a/source/common/network/connection_balancer_impl.cc b/source/common/network/connection_balancer_impl.cc index 309628073f19..fc0471c675a5 100644 --- a/source/common/network/connection_balancer_impl.cc +++ b/source/common/network/connection_balancer_impl.cc @@ -28,7 +28,7 @@ ExactConnectionBalancerImpl::pickTargetHandler(BalancedConnectionHandler&) { } } - min_connection_handler->incNumConnections(); + min_connection_handler->incNumConnections(); // NOLINT(clang-analyzer-core.CallAndMessage) } return *min_connection_handler; diff --git a/source/common/quic/envoy_quic_server_connection.h b/source/common/quic/envoy_quic_server_connection.h index 468257decfe8..5d5de00ef5c5 100644 --- a/source/common/quic/envoy_quic_server_connection.h +++ b/source/common/quic/envoy_quic_server_connection.h @@ -78,6 +78,7 @@ class QuicListenerFilterManagerImpl : public Network::QuicListenerFilterManager, } bool shouldAdvertiseServerPreferredAddress( const quic::QuicSocketAddress& server_preferred_address) const override { + // NOLINTNEXTLINE(modernize-loop-convert) for (auto iter = accept_filters_.begin(); iter != accept_filters_.end(); iter++) { if (!(*iter)->isCompatibleWithServerPreferredAddress(server_preferred_address)) { return false; @@ -87,6 +88,7 @@ class QuicListenerFilterManagerImpl : public Network::QuicListenerFilterManager, } void onPeerAddressChanged(const quic::QuicSocketAddress& new_address, Network::Connection& connection) override { + // NOLINTNEXTLINE(modernize-loop-convert) for (auto iter = accept_filters_.begin(); iter != accept_filters_.end(); iter++) { Network::FilterStatus status = (*iter)->onPeerAddressChanged(new_address, connection); if (status == Network::FilterStatus::StopIteration || @@ -96,6 +98,7 @@ class QuicListenerFilterManagerImpl : public Network::QuicListenerFilterManager, } } void startFilterChain() { + // NOLINTNEXTLINE(modernize-loop-convert) for (auto iter = accept_filters_.begin(); iter != accept_filters_.end(); iter++) { Network::FilterStatus status = (*iter)->onAccept(*this); if (status == Network::FilterStatus::StopIteration || !socket().ioHandle().isOpen()) { diff --git a/source/common/rds/rds_route_config_subscription.cc b/source/common/rds/rds_route_config_subscription.cc index 47acc81ce85f..44d2e15d59a6 100644 --- a/source/common/rds/rds_route_config_subscription.cc +++ b/source/common/rds/rds_route_config_subscription.cc @@ -52,7 +52,7 @@ RdsRouteConfigSubscription::~RdsRouteConfigSubscription() { absl::Status RdsRouteConfigSubscription::onConfigUpdate( const std::vector& resources, const std::string& version_info) { - if (resources.size() == 0) { + if (resources.empty()) { ENVOY_LOG(debug, "Missing {} RouteConfiguration for {} in onConfigUpdate()", rds_type_, route_config_name_); stats_.update_empty_.inc(); diff --git a/source/common/stats/symbol_table.h b/source/common/stats/symbol_table.h index f7ad027eeeca..5f79985ae4f2 100644 --- a/source/common/stats/symbol_table.h +++ b/source/common/stats/symbol_table.h @@ -702,7 +702,7 @@ class StatNameManagedStorage : public StatNameStorage { StatNameManagedStorage(StatName src, SymbolTable& table) noexcept : StatNameStorage(src, table), symbol_table_(table) {} - ~StatNameManagedStorage() { free(symbol_table_); } + ~StatNameManagedStorage() { free(symbol_table_); } // NOLINT(clang-analyzer-unix.Malloc) private: SymbolTable& symbol_table_; diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 9f40a915cc9b..09e235e6ac4c 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -258,7 +258,7 @@ ThreadLocalStoreImpl::CentralCacheEntry::~CentralCacheEntry() { // is because many tests will not populate rejected_stats_. ASSERT(symbol_table_.toString(StatNameManagedStorage("Hello.world", symbol_table_).statName()) == "Hello.world"); - rejected_stats_.free(symbol_table_); + rejected_stats_.free(symbol_table_); // NOLINT(clang-analyzer-unix.Malloc) } void ThreadLocalStoreImpl::releaseScopeCrossThread(ScopeImpl* scope) { diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc index 72a76e7df0d8..15bfa82596c4 100644 --- a/source/common/upstream/load_balancer_impl.cc +++ b/source/common/upstream/load_balancer_impl.cc @@ -232,8 +232,10 @@ void LoadBalancerBase::recalculatePerPriorityState(uint32_t priority, // all hosts are healthy that priority's health is 100%*1.4=140% and is capped at 100% which // results in 100%. If 80% of hosts are healthy, that priority's health is still 100% // (80%*1.4=112% and capped at 100%). - per_priority_health.get()[priority] = std::min( - 100, (host_set.overprovisioningFactor() * healthy_weight / total_weight)); + per_priority_health.get()[priority] = + std::min(100, + // NOLINTNEXTLINE(clang-analyzer-core.DivideZero) + (host_set.overprovisioningFactor() * healthy_weight / total_weight)); // We perform the same computation for degraded hosts. per_priority_degraded.get()[priority] = std::min( diff --git a/source/extensions/clusters/eds/eds.cc b/source/extensions/clusters/eds/eds.cc index 58d2fceca827..bfb6670846be 100644 --- a/source/extensions/clusters/eds/eds.cc +++ b/source/extensions/clusters/eds/eds.cc @@ -163,7 +163,7 @@ void EdsClusterImpl::BatchUpdateHelper::updateLocalityEndpoints( absl::Status EdsClusterImpl::onConfigUpdate(const std::vector& resources, const std::string&) { - if (resources.size() == 0) { + if (resources.empty()) { ENVOY_LOG(debug, "Missing ClusterLoadAssignment for {} in onConfigUpdate()", edsServiceName()); info_->configUpdateStats().update_empty_.inc(); onPreInitComplete(); diff --git a/source/server/hot_restarting_base.cc b/source/server/hot_restarting_base.cc index 69084b50e187..af9bf4cd29e2 100644 --- a/source/server/hot_restarting_base.cc +++ b/source/server/hot_restarting_base.cc @@ -157,6 +157,7 @@ bool RpcStream::replyIsExpectedType(const HotRestartMessage* proto, // should only get control data in a PassListenSocketReply, it should only be the fd passing type, // and there should only be one at a time. Crash on any other control data. void RpcStream::getPassedFdIfPresent(HotRestartMessage* out, msghdr* message) { + // NOLINTNEXTLINE(clang-analyzer-core.UndefinedBinaryOperatorResult) cmsghdr* cmsg = CMSG_FIRSTHDR(message); if (cmsg != nullptr) { RELEASE_ASSERT(cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS && diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc index dc4b223535de..62268d2d84d7 100644 --- a/source/server/options_impl.cc +++ b/source/server/options_impl.cc @@ -54,6 +54,7 @@ OptionsImpl::OptionsImpl(std::vector args, "\nDefault is \"{}\"", Logger::Logger::DEFAULT_LOG_FORMAT); + // NOLINTNEXTLINE(clang-analyzer-optin.cplusplus.VirtualCall) TCLAP::CmdLine cmd("envoy", ' ', VersionInfo::version()); TCLAP::ValueArg base_id( "", "base-id", "Base ID so that multiple envoys can run on the same host if needed", false, 0, diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc index 90bdbb10e670..8c0f0c764382 100644 --- a/test/benchmark/main.cc +++ b/test/benchmark/main.cc @@ -26,14 +26,14 @@ int main(int argc, char** argv) { bool contains_help_flag = false; // Checking if any of the command-line arguments contains `--help` - for (int i = 1; i < argc; ++i) { + for (int i = 1; i < argc; ++i) { // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall) if (strcmp(argv[i], "--help") == 0) { contains_help_flag = true; break; } } - if (contains_help_flag) { + if (contains_help_flag) { // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall) // if the `--help` flag isn't considered separately, it runs "benchmark --help" // (Google Benchmark Help) and the help output doesn't contains details about // custom defined flags like `--skip_expensive_benchmarks`, `--runtime_feature`, etc diff --git a/test/common/access_log/access_log_manager_impl_test.cc b/test/common/access_log/access_log_manager_impl_test.cc index 3bacc57c0bd9..e3debf6d851a 100644 --- a/test/common/access_log/access_log_manager_impl_test.cc +++ b/test/common/access_log/access_log_manager_impl_test.cc @@ -233,6 +233,7 @@ TEST_F(AccessLogManagerImplTest, FlushCountsIOErrors) { } TEST_F(AccessLogManagerImplTest, ReopenFile) { + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) NiceMock* timer = new NiceMock(&dispatcher_); Sequence sq; @@ -275,12 +276,14 @@ TEST_F(AccessLogManagerImplTest, ReopenFile) { log_file->write("reopened"); timer->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) EXPECT_TRUE(file_->waitForEventCount(file_->num_writes_, 2)); EXPECT_TRUE(file_->waitForEventCount(file_->num_opens_, 2)); } // Test that the `reopen()` will trigger file reopen even if no data is waiting. TEST_F(AccessLogManagerImplTest, ReopenFileNoWrite) { + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) NiceMock* timer = new NiceMock(&dispatcher_); Sequence sq; @@ -299,6 +302,7 @@ TEST_F(AccessLogManagerImplTest, ReopenFileNoWrite) { log_file->write("before"); timer->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) EXPECT_TRUE(file_->waitForEventCount(file_->num_writes_, 1)); EXPECT_CALL(*file_, close_()) @@ -318,6 +322,7 @@ TEST_F(AccessLogManagerImplTest, ReopenFileNoWrite) { } TEST_F(AccessLogManagerImplTest, ReopenRetry) { + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) NiceMock* timer = new NiceMock(&dispatcher_); Sequence sq; @@ -384,6 +389,7 @@ TEST_F(AccessLogManagerImplTest, ReopenRetry) { log_file->write("after reopen"); timer->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) EXPECT_TRUE(file_->waitForEventCount(file_->num_writes_, 3)); waitForCounterEq("filesystem.reopen_failed", 2); waitForGaugeEq("filesystem.write_total_buffered", 0); diff --git a/test/common/common/utility_speed_test.cc b/test/common/common/utility_speed_test.cc index ce9d8c790330..1fbfb91fcd96 100644 --- a/test/common/common/utility_speed_test.cc +++ b/test/common/common/utility_speed_test.cc @@ -22,7 +22,7 @@ static size_t CacheControlLength = sizeof(CacheControl) - 1; // NOLINT(namespace-envoy) -static void BM_AccessLogDateTimeFormatter(benchmark::State& state) { +static void bmAccessLogDateTimeFormatter(benchmark::State& state) { int outputBytes = 0; // Generate a sequence of times for which the delta between each successive @@ -32,7 +32,7 @@ static void BM_AccessLogDateTimeFormatter(benchmark::State& state) { static Envoy::SystemTime time(std::chrono::seconds(1522796769)); static std::mt19937 prng(1); // PRNG with a fixed seed, for repeatability static std::uniform_int_distribution distribution(-10, 20); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) // TODO(brian-pane): The next line, which computes the next input timestamp, // currently accounts for ~30% of the CPU time of this benchmark test. If // the AccessLogDateTimeFormatter implementation is optimized further, we @@ -43,30 +43,30 @@ static void BM_AccessLogDateTimeFormatter(benchmark::State& state) { } benchmark::DoNotOptimize(outputBytes); } -BENCHMARK(BM_AccessLogDateTimeFormatter); +BENCHMARK(bmAccessLogDateTimeFormatter); -// This benchmark is basically similar with the above BM_AccessLogDateTimeFormatter, the only +// This benchmark is basically similar with the above bmAccessLogDateTimeFormatter, the only // difference is the format string input for the Envoy::DateFormatter. -static void BM_DateTimeFormatterWithSubseconds(benchmark::State& state) { +static void bmDateTimeFormatterWithSubseconds(benchmark::State& state) { int outputBytes = 0; Envoy::SystemTime time(std::chrono::seconds(1522796769)); std::mt19937 prng(1); std::uniform_int_distribution distribution(-10, 20); Envoy::DateFormatter date_formatter("%Y-%m-%dT%H:%M:%s.%3f"); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) time += std::chrono::milliseconds(static_cast(distribution(prng))); outputBytes += date_formatter.fromTime(time).length(); } benchmark::DoNotOptimize(outputBytes); } -BENCHMARK(BM_DateTimeFormatterWithSubseconds); +BENCHMARK(bmDateTimeFormatterWithSubseconds); -// This benchmark is basically similar with the above BM_DateTimeFormatterWithSubseconds, the +// This benchmark is basically similar with the above bmDateTimeFormatterWithSubseconds, the // differences are: 1. the format string input is long with duplicated subseconds. 2. The purpose // is to test DateFormatter.parse() which is called in constructor. // NOLINTNEXTLINE(readability-identifier-naming) -static void BM_DateTimeFormatterWithLongSubsecondsString(benchmark::State& state) { +static void bmDateTimeFormatterWithLongSubsecondsString(benchmark::State& state) { int outputBytes = 0; Envoy::SystemTime time(std::chrono::seconds(1522796769)); @@ -80,71 +80,71 @@ static void BM_DateTimeFormatterWithLongSubsecondsString(benchmark::State& state } absl::StrAppend(&input, duplicate_input); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::DateFormatter date_formatter(input); time += std::chrono::milliseconds(static_cast(distribution(prng))); outputBytes += date_formatter.fromTime(time).length(); } benchmark::DoNotOptimize(outputBytes); } -BENCHMARK(BM_DateTimeFormatterWithLongSubsecondsString); +BENCHMARK(bmDateTimeFormatterWithLongSubsecondsString); // NOLINTNEXTLINE(readability-identifier-naming) -static void BM_DateTimeFormatterWithoutSubseconds(benchmark::State& state) { +static void bmDateTimeFormatterWithoutSubseconds(benchmark::State& state) { int outputBytes = 0; Envoy::SystemTime time(std::chrono::seconds(1522796769)); std::mt19937 prng(1); std::uniform_int_distribution distribution(-10, 20); Envoy::DateFormatter date_formatter("%Y-%m-%dT%H:%M:%s"); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) time += std::chrono::milliseconds(static_cast(distribution(prng))); outputBytes += date_formatter.fromTime(time).length(); } benchmark::DoNotOptimize(outputBytes); } -BENCHMARK(BM_DateTimeFormatterWithoutSubseconds); +BENCHMARK(bmDateTimeFormatterWithoutSubseconds); -static void BM_RTrimStringView(benchmark::State& state) { +static void bmRTrimStringView(benchmark::State& state) { int accum = 0; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) absl::string_view text(TextToTrim, TextToTrimLength); text = Envoy::StringUtil::rtrim(text); accum += TextToTrimLength - text.size(); } benchmark::DoNotOptimize(accum); } -BENCHMARK(BM_RTrimStringView); +BENCHMARK(bmRTrimStringView); -static void BM_RTrimStringViewAlreadyTrimmed(benchmark::State& state) { +static void bmRTrimStringViewAlreadyTrimmed(benchmark::State& state) { int accum = 0; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) absl::string_view text(AlreadyTrimmed, AlreadyTrimmedLength); text = Envoy::StringUtil::rtrim(text); accum += AlreadyTrimmedLength - text.size(); } benchmark::DoNotOptimize(accum); } -BENCHMARK(BM_RTrimStringViewAlreadyTrimmed); +BENCHMARK(bmRTrimStringViewAlreadyTrimmed); -static void BM_RTrimStringViewAlreadyTrimmedAndMakeString(benchmark::State& state) { +static void bmRTrimStringViewAlreadyTrimmedAndMakeString(benchmark::State& state) { int accum = 0; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) absl::string_view text(AlreadyTrimmed, AlreadyTrimmedLength); std::string string_copy = std::string(Envoy::StringUtil::rtrim(text)); accum += AlreadyTrimmedLength - string_copy.size(); } benchmark::DoNotOptimize(accum); } -BENCHMARK(BM_RTrimStringViewAlreadyTrimmedAndMakeString); +BENCHMARK(bmRTrimStringViewAlreadyTrimmedAndMakeString); -static void BM_FindToken(benchmark::State& state) { +static void bmFindToken(benchmark::State& state) { const absl::string_view cache_control(CacheControl, CacheControlLength); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) RELEASE_ASSERT(Envoy::StringUtil::findToken(cache_control, ",", "no-transform"), ""); } } -BENCHMARK(BM_FindToken); +BENCHMARK(bmFindToken); static bool nextToken(absl::string_view& str, char delim, bool strip_whitespace, absl::string_view* token) { @@ -180,18 +180,18 @@ static bool findTokenWithoutSplitting(absl::string_view str, char delim, absl::s return false; } -static void BM_FindTokenWithoutSplitting(benchmark::State& state) { +static void bmFindTokenWithoutSplitting(benchmark::State& state) { const absl::string_view cache_control(CacheControl, CacheControlLength); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) RELEASE_ASSERT(findTokenWithoutSplitting(cache_control, ',', "no-transform", true), ""); } } -BENCHMARK(BM_FindTokenWithoutSplitting); +BENCHMARK(bmFindTokenWithoutSplitting); -static void BM_FindTokenValueNestedSplit(benchmark::State& state) { +static void bmFindTokenValueNestedSplit(benchmark::State& state) { const absl::string_view cache_control(CacheControl, CacheControlLength); absl::string_view max_age; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) for (absl::string_view token : Envoy::StringUtil::splitToken(cache_control, ",")) { auto name_value = Envoy::StringUtil::splitToken(token, "="); if ((name_value.size() == 2) && (Envoy::StringUtil::trim(name_value[0]) == "max-age")) { @@ -201,10 +201,10 @@ static void BM_FindTokenValueNestedSplit(benchmark::State& state) { RELEASE_ASSERT(max_age == "300", ""); } } -BENCHMARK(BM_FindTokenValueNestedSplit); +BENCHMARK(bmFindTokenValueNestedSplit); -static void BM_FindTokenValueSearchForEqual(benchmark::State& state) { - for (auto _ : state) { +static void bmFindTokenValueSearchForEqual(benchmark::State& state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) const absl::string_view cache_control(CacheControl, CacheControlLength); absl::string_view max_age; for (absl::string_view token : Envoy::StringUtil::splitToken(cache_control, ",")) { @@ -217,10 +217,10 @@ static void BM_FindTokenValueSearchForEqual(benchmark::State& state) { RELEASE_ASSERT(max_age == "300", ""); } } -BENCHMARK(BM_FindTokenValueSearchForEqual); +BENCHMARK(bmFindTokenValueSearchForEqual); -static void BM_FindTokenValueNoSplit(benchmark::State& state) { - for (auto _ : state) { +static void bmFindTokenValueNoSplit(benchmark::State& state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) absl::string_view cache_control(CacheControl, CacheControlLength); absl::string_view max_age; for (absl::string_view token; nextToken(cache_control, ',', true, &token);) { @@ -232,9 +232,9 @@ static void BM_FindTokenValueNoSplit(benchmark::State& state) { RELEASE_ASSERT(max_age == "300", ""); } } -BENCHMARK(BM_FindTokenValueNoSplit); +BENCHMARK(bmFindTokenValueNoSplit); -static void BM_RemoveTokensLong(benchmark::State& state) { +static void bmRemoveTokensLong(benchmark::State& state) { auto size = state.range(0); std::string input(size, ','); std::vector to_remove; @@ -249,15 +249,15 @@ static void BM_RemoveTokensLong(benchmark::State& state) { input.append(","); input.append(to_remove[i]); } - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::StringUtil::removeTokens(input, ",", to_remove_set, ","); state.SetBytesProcessed(static_cast(state.iterations()) * input.size()); } } -BENCHMARK(BM_RemoveTokensLong)->Range(8, 8 << 10); +BENCHMARK(bmRemoveTokensLong)->Range(8, 8 << 10); -static void BM_IntervalSetInsert17(benchmark::State& state) { - for (auto _ : state) { +static void bmIntervalSetInsert17(benchmark::State& state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::IntervalSetImpl interval_set; interval_set.insert(7, 10); interval_set.insert(-2, -1); @@ -278,28 +278,28 @@ static void BM_IntervalSetInsert17(benchmark::State& state) { interval_set.insert(24, 9223372036854775805UL); } } -BENCHMARK(BM_IntervalSetInsert17); +BENCHMARK(bmIntervalSetInsert17); -static void BM_IntervalSet4ToVector(benchmark::State& state) { +static void bmIntervalSet4ToVector(benchmark::State& state) { Envoy::IntervalSetImpl interval_set; interval_set.insert(7, 10); interval_set.insert(-2, -1); interval_set.insert(22, 23); interval_set.insert(8, 15); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) benchmark::DoNotOptimize(interval_set.toVector()); } } -BENCHMARK(BM_IntervalSet4ToVector); +BENCHMARK(bmIntervalSet4ToVector); -static void BM_IntervalSet50ToVector(benchmark::State& state) { +static void bmIntervalSet50ToVector(benchmark::State& state) { Envoy::IntervalSetImpl interval_set; for (size_t i = 0; i < 100; i += 2) { interval_set.insert(i, i + 1); } - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) benchmark::DoNotOptimize(interval_set.toVector()); } } -BENCHMARK(BM_IntervalSet50ToVector); +BENCHMARK(bmIntervalSet50ToVector); } // namespace Envoy diff --git a/test/common/grpc/async_client_manager_benchmark.cc b/test/common/grpc/async_client_manager_benchmark.cc index f23a0aba54e5..8498a59e8668 100644 --- a/test/common/grpc/async_client_manager_benchmark.cc +++ b/test/common/grpc/async_client_manager_benchmark.cc @@ -46,7 +46,7 @@ void testGetOrCreateAsyncClientWithConfig(::benchmark::State& state) { envoy::config::core::v3::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) for (int i = 0; i < 1000; i++) { RawAsyncClientSharedPtr foo_client0 = async_client_man_test.async_client_manager_.getOrCreateRawAsyncClient( @@ -62,7 +62,7 @@ void testGetOrCreateAsyncClientWithHashConfig(::benchmark::State& state) { grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); GrpcServiceConfigWithHashKey config_with_hash_key_a = GrpcServiceConfigWithHashKey(grpc_service); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) for (int i = 0; i < 1000; i++) { RawAsyncClientSharedPtr foo_client0 = async_client_man_test.async_client_manager_.getOrCreateRawAsyncClientWithHashKey( diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index f42d7ed316d1..1ccfd8202c2e 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -30,7 +30,7 @@ using testing::_; using testing::AtMost; -using testing::ByMove; +using testing::ByMove; // NOLINT(misc-unused-using-decls) using testing::Invoke; using testing::InvokeWithoutArgs; using testing::NiceMock; diff --git a/test/common/http/codes_speed_test.cc b/test/common/http/codes_speed_test.cc index ec82b9546c54..275871d86a04 100644 --- a/test/common/http/codes_speed_test.cc +++ b/test/common/http/codes_speed_test.cc @@ -97,7 +97,7 @@ template class CodeUtilitySpeedTest { static void BM_AddResponsesRealSymtab(benchmark::State& state) { Envoy::Http::CodeUtilitySpeedTest context; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) context.addResponses(); } } @@ -107,7 +107,7 @@ BENCHMARK(BM_AddResponsesRealSymtab); static void BM_ResponseTimingRealSymtab(benchmark::State& state) { Envoy::Http::CodeUtilitySpeedTest context; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) context.responseTiming(); } } diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 60dc4ad1fc89..d8e913d18a70 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -4641,7 +4641,8 @@ TEST_P(Http1ServerConnectionImplTest, SeparatorInHeaderName) { // BalsaParser always rejects a header name with space. HttpParser only rejects // it in strict mode, which is disabled when ENVOY_ENABLE_UHV is defined. TEST_P(Http1ClientConnectionImplTest, SpaceInHeaderName) { - bool accept = parser_impl_ == Http1ParserImpl::HttpParser; + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) + bool accept = (parser_impl_ == Http1ParserImpl::HttpParser); #ifndef ENVOY_ENABLE_UHV accept = false; #endif @@ -4675,7 +4676,8 @@ TEST_P(Http1ClientConnectionImplTest, SpaceInHeaderName) { } TEST_P(Http1ServerConnectionImplTest, SpaceInHeaderName) { - bool accept = parser_impl_ == Http1ParserImpl::HttpParser; + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) + bool accept = (parser_impl_ == Http1ParserImpl::HttpParser); #ifndef ENVOY_ENABLE_UHV accept = false; #endif diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 4a8c0e0a82a4..6eec4fc0b1c3 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -1645,6 +1645,7 @@ TEST_P(Http2CodecImplTest, ShouldRestoreCrashDumpInfoWhenHandlingDeferredProcess process_buffered_data_callback->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) EXPECT_THAT(ostream.contents(), HasSubstr("Http2::ConnectionImpl ")); EXPECT_THAT(ostream.contents(), HasSubstr("Dumping current stream:\n stream: \n ConnectionImpl::StreamImpl")); @@ -4016,6 +4017,7 @@ TEST_P(Http2CodecImplTest, InSequence seq; EXPECT_CALL(request_decoder_, decodeTrailers_(_)); process_buffered_data_callback->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) EXPECT_FALSE(process_buffered_data_callback->enabled_); } } @@ -4162,6 +4164,7 @@ TEST_P(Http2CodecImplTest, ChunksLargeBodyDuringDeferredProcessing) { EXPECT_CALL(request_decoder_, decodeData(_, true)); process_buffered_data_callback->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) EXPECT_FALSE(process_buffered_data_callback->enabled_); } } diff --git a/test/common/http/http3/conn_pool_test.cc b/test/common/http/http3/conn_pool_test.cc index d27e0b8fd92b..14e110f7d47f 100644 --- a/test/common/http/http3/conn_pool_test.cc +++ b/test/common/http/http3/conn_pool_test.cc @@ -172,6 +172,7 @@ TEST_F(Http3ConnPoolImplTest, CreationAndNewStream) { })); EXPECT_CALL(*cluster_socket_option, setOption(_, _)).Times(3u); EXPECT_CALL(*socket_option_, setOption(_, _)).Times(3u); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) auto* async_connect_callback = new NiceMock(&dispatcher_); ConnectionPool::Cancellable* cancellable = pool_->newStream(decoder, callbacks, {/*can_send_early_data_=*/false, @@ -179,6 +180,7 @@ TEST_F(Http3ConnPoolImplTest, CreationAndNewStream) { EXPECT_NE(nullptr, cancellable); async_connect_callback->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) std::list& clients = Http3ConnPoolImplPeer::connectingClients(*pool_); EXPECT_EQ(1u, clients.size()); diff --git a/test/common/network/address_impl_speed_test.cc b/test/common/network/address_impl_speed_test.cc index 9eea883a3cf9..fcf0a3e60cdc 100644 --- a/test/common/network/address_impl_speed_test.cc +++ b/test/common/network/address_impl_speed_test.cc @@ -14,7 +14,7 @@ static void ipv4InstanceCreate(benchmark::State& state) { addr.sin_port = htons(443); static constexpr uint32_t Addr = 0xc00002ff; // From the RFC 5737 example range. addr.sin_addr.s_addr = htonl(Addr); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Ipv4Instance address(&addr); benchmark::DoNotOptimize(address.ip()); } @@ -28,7 +28,7 @@ static void ipv6InstanceCreate(benchmark::State& state) { addr.sin6_port = htons(443); static const char* Addr = "2001:DB8::1234"; // From the RFC 3849 example range. inet_pton(AF_INET6, Addr, &addr.sin6_addr); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Ipv6Instance address(addr); benchmark::DoNotOptimize(address.ip()); } diff --git a/test/common/network/lc_trie_speed_test.cc b/test/common/network/lc_trie_speed_test.cc index 7a524fe0f135..a245baad0383 100644 --- a/test/common/network/lc_trie_speed_test.cc +++ b/test/common/network/lc_trie_speed_test.cc @@ -58,7 +58,7 @@ static void lcTrieConstruct(benchmark::State& state) { CidrInputs inputs; std::unique_ptr> trie; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) trie = std::make_unique>(inputs.tag_data_); } benchmark::DoNotOptimize(trie); @@ -70,7 +70,7 @@ static void lcTrieConstructNested(benchmark::State& state) { CidrInputs inputs; std::unique_ptr> trie; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) trie = std::make_unique>( inputs.tag_data_nested_prefixes_); } @@ -83,7 +83,8 @@ static void lcTrieConstructMinimal(benchmark::State& state) { CidrInputs inputs; std::unique_ptr> trie; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) trie = std::make_unique>(inputs.tag_data_minimal_); } benchmark::DoNotOptimize(trie); @@ -99,7 +100,7 @@ static void lcTrieLookup(benchmark::State& state) { static size_t i = 0; size_t output_tags = 0; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) i++; i %= address_inputs.addresses_.size(); output_tags += lc_trie->getData(address_inputs.addresses_[i]).size(); @@ -118,7 +119,7 @@ static void lcTrieLookupWithNestedPrefixes(benchmark::State& state) { static size_t i = 0; size_t output_tags = 0; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) i++; i %= address_inputs.addresses_.size(); output_tags += lc_trie_nested_prefixes->getData(address_inputs.addresses_[i]).size(); @@ -136,7 +137,7 @@ static void lcTrieLookupMinimal(benchmark::State& state) { static size_t i = 0; size_t output_tags = 0; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) i++; i %= address_inputs.addresses_.size(); output_tags += lc_trie_minimal->getData(address_inputs.addresses_[i]).size(); diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 04957d0e2905..f6f7423c9fce 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -380,6 +380,7 @@ ZCFbredVxDBZuoVsfrKPSQa407Jj1Q== } TEST_F(EnvoyQuicProofVerifierTest, VerifySubjectAltNameListOverrideFailure) { + // NOLINTNEXTLINE(modernize-make-shared) transport_socket_options_.reset(new Network::TransportSocketOptionsImpl("", {"non-example.com"})); configCertVerificationDetails(true); std::unique_ptr cert_view = diff --git a/test/common/stats/symbol_table_impl_test.cc b/test/common/stats/symbol_table_impl_test.cc index e74b52a80607..e8c95391e308 100644 --- a/test/common/stats/symbol_table_impl_test.cc +++ b/test/common/stats/symbol_table_impl_test.cc @@ -287,7 +287,7 @@ TEST_F(StatNameTest, TestSameValueOnPartialFree) { StatNameStorage stat_foobar_1("foo.bar", table_); SymbolVec stat_foobar_1_symbols = getSymbols(stat_foobar_1.statName()); stat_foobar_1.free(table_); - StatName stat_foobar_2(makeStat("foo.bar")); + StatName stat_foobar_2(makeStat("foo.bar")); // NOLINT(clang-analyzer-unix.Malloc) SymbolVec stat_foobar_2_symbols = getSymbols(stat_foobar_2); EXPECT_EQ(stat_foobar_1_symbols[0], @@ -337,7 +337,7 @@ TEST_F(StatNameTest, TestShrinkingExpectation) { size_t table_size_0 = table_.numSymbols(); auto make_stat_storage = [this](absl::string_view name) -> StatNameStorage { - return StatNameStorage(name, table_); + return {name, table_}; }; StatNameStorage stat_a(make_stat_storage("a")); @@ -362,7 +362,7 @@ TEST_F(StatNameTest, TestShrinkingExpectation) { stat_ace.free(table_); EXPECT_EQ(table_size_4, table_.numSymbols()); - stat_acd.free(table_); + stat_acd.free(table_); // NOLINT(clang-analyzer-unix.Malloc) EXPECT_EQ(table_size_3, table_.numSymbols()); stat_ac.free(table_); @@ -586,7 +586,7 @@ TEST_F(StatNameTest, MutexContentionOnExistingSymbols) { accesses.DecrementCount(); wait.wait(); - })); + })); // NOLINT(clang-analyzer-unix.Malloc) } creation.setReady(); creates.Wait(); @@ -744,7 +744,7 @@ TEST(SymbolTableTest, Memory) { }; symbol_table_mem_used = test_memory_usage(record_stat); for (StatNameStorage& name : names) { - name.free(table); + name.free(table); // NOLINT(clang-analyzer-unix.Malloc) } } diff --git a/test/common/stats/symbol_table_speed_test.cc b/test/common/stats/symbol_table_speed_test.cc index e98eefa89bd7..2e6f084a07de 100644 --- a/test/common/stats/symbol_table_speed_test.cc +++ b/test/common/stats/symbol_table_speed_test.cc @@ -41,6 +41,7 @@ static void bmCreateRace(benchmark::State& state) { access.wait(); for (int count = 0; count < 1000; ++count) { + // NOLINTNEXTLINE(clang-analyzer-unix.Malloc) Envoy::Stats::StatNameStorage second(stat_name_string, table); second.free(table); } diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index adf91036c986..8ff2b11aa25d 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -2643,6 +2643,7 @@ TEST_P(ClusterManagerLifecycleTest, DynamicHostRemove) { // drain callbacks, etc. dns_timer_->invokeCallback(); dns_callback(Network::DnsResolver::ResolutionStatus::Success, + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) TestUtility::makeDnsResponse({"127.0.0.2", "127.0.0.3"})); factory_.tls_.shutdownThread(); } @@ -2864,6 +2865,7 @@ TEST_P(ClusterManagerLifecycleTest, DynamicHostRemoveWithTls) { // drain callbacks, etc. dns_timer_->invokeCallback(); dns_callback(Network::DnsResolver::ResolutionStatus::Success, + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) TestUtility::makeDnsResponse({"127.0.0.2", "127.0.0.3"})); factory_.tls_.shutdownThread(); } @@ -3684,6 +3686,7 @@ TEST_P(ClusterManagerLifecycleTest, DynamicHostRemoveDefaultPriority) { // Remove the first host, this should lead to the cp being drained, without // crash. dns_timer_->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) dns_callback(Network::DnsResolver::ResolutionStatus::Success, TestUtility::makeDnsResponse({})); factory_.tls_.shutdownThread(); @@ -3768,6 +3771,7 @@ TEST_P(ClusterManagerLifecycleTest, ConnPoolDestroyWithDraining) { // Remove the first host, this should lead to the cp being drained. dns_timer_->invokeCallback(); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) dns_callback(Network::DnsResolver::ResolutionStatus::Success, TestUtility::makeDnsResponse({})); // The drained callback might get called when the CP is being destroyed. diff --git a/test/common/upstream/default_local_address_selector_test.cc b/test/common/upstream/default_local_address_selector_test.cc index ba2d54cd381b..a5a1d5536346 100644 --- a/test/common/upstream/default_local_address_selector_test.cc +++ b/test/common/upstream/default_local_address_selector_test.cc @@ -31,7 +31,7 @@ TEST(ConfigTest, NullUpstreamAddress) { // This should be exception free. UpstreamLocalAddressSelectorConstSharedPtr selector = factory.createLocalAddressSelector(upstream_local_addresses, absl::nullopt); -} +} // NOLINT(clang-analyzer-cplusplus.NewDeleteLeaks) } // namespace } // namespace Upstream diff --git a/test/extensions/common/wasm/test_data/test_context_cpp.cc b/test/extensions/common/wasm/test_data/test_context_cpp.cc index 225fda61b8c5..02d9264cedc5 100644 --- a/test/extensions/common/wasm/test_data/test_context_cpp.cc +++ b/test/extensions/common/wasm/test_data/test_context_cpp.cc @@ -106,7 +106,7 @@ class PanicReplyContext : public Context { FilterDataStatus PanicReplyContext::onRequestBody(size_t, bool) { sendLocalResponse(200, "not send", "body", {}); int* badptr = nullptr; - *badptr = 0; + *badptr = 0; // NOLINT(clang-analyzer-core.NullDereference) return FilterDataStatus::Continue; } diff --git a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc index e5f9a74b1a35..5ce46084511f 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc @@ -27,7 +27,8 @@ using Headers = std::vector>; class ExtAuthzGrpcIntegrationTest : public Grpc::GrpcClientIntegrationParamTest, public HttpIntegrationTest { public: - ExtAuthzGrpcIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, ipVersion()) {} + ExtAuthzGrpcIntegrationTest() + : HttpIntegrationTest(Http::CodecType::HTTP1, ExtAuthzGrpcIntegrationTest::ipVersion()) {} void createUpstreams() override { HttpIntegrationTest::createUpstreams(); diff --git a/test/extensions/filters/http/wasm/test_data/test_cpp.cc b/test/extensions/filters/http/wasm/test_data/test_cpp.cc index 3ddb83924630..2bb3b9d1f8eb 100644 --- a/test/extensions/filters/http/wasm/test_data/test_cpp.cc +++ b/test/extensions/filters/http/wasm/test_data/test_cpp.cc @@ -280,7 +280,7 @@ FilterHeadersStatus TestContext::onRequestHeaders(uint32_t, bool) { FilterTrailersStatus TestContext::onRequestTrailers(uint32_t) { auto request_trailer = getRequestTrailer("bogus-trailer"); - if (request_trailer && request_trailer->view() != "") { + if (request_trailer && !request_trailer->view().empty()) { logWarn("request bogus-trailer found"); } CHECK_RESULT(replaceRequestTrailer("new-trailer", "value")); @@ -288,7 +288,7 @@ FilterTrailersStatus TestContext::onRequestTrailers(uint32_t) { // Not available yet. replaceResponseTrailer("new-trailer", "value"); auto response_trailer = getResponseTrailer("bogus-trailer"); - if (response_trailer && response_trailer->view() != "") { + if (response_trailer && !response_trailer->view().empty()) { logWarn("request bogus-trailer found"); } return FilterTrailersStatus::Continue; @@ -305,7 +305,7 @@ FilterHeadersStatus TestContext::onResponseHeaders(uint32_t, bool) { FilterTrailersStatus TestContext::onResponseTrailers(uint32_t) { auto value = getResponseTrailer("bogus-trailer"); - if (value && value->view() != "") { + if (value && !value->view().empty()) { logWarn("response bogus-trailer found"); } CHECK_RESULT(replaceResponseTrailer("new-trailer", "value")); @@ -362,15 +362,15 @@ void TestContext::onLog() { logWarn("onLog " + std::to_string(id()) + " " + std::string(path->view()) + " " + std::string(status->view())); auto response_header = getResponseHeader("bogus-header"); - if (response_header && response_header->view() != "") { + if (response_header && !response_header->view().empty()) { logWarn("response bogus-header found"); } auto response_trailer = getResponseTrailer("bogus-trailer"); - if (response_trailer && response_trailer->view() != "") { + if (response_trailer && !response_trailer->view().empty()) { logWarn("response bogus-trailer found"); } auto request_trailer = getRequestTrailer("error-details"); - if (request_trailer && request_trailer->view() != "") { + if (request_trailer && !request_trailer->view().empty()) { logWarn("request bogus-trailer found"); } } else if (test == "cluster_metadata") { @@ -537,7 +537,7 @@ void TestContext::onLog() { // validate null field std::string b; - if (!getValue({"protobuf_state", "b"}, &b) || b != "") { + if (!getValue({"protobuf_state", "b"}, &b) || !b.empty()) { logWarn("null field returned " + b); } @@ -618,16 +618,16 @@ void TestContext::onDone() { } void TestRootContext::onTick() { - if (test_ == "headers") { + if (test_ == "headers") { // NOLINT(clang-analyzer-optin.portability.UnixAPI) getContext(stream_context_id_)->setEffectiveContext(); replaceRequestHeader("server", "envoy-wasm-continue"); continueRequest(); - if (getBufferBytes(WasmBufferType::PluginConfiguration, 0, 1)->view() != "") { + if (!getBufferBytes(WasmBufferType::PluginConfiguration, 0, 1)->view().empty()) { logDebug("unexpectd success of getBufferBytes PluginConfiguration"); } - } else if (test_ == "metadata") { + } else if (test_ == "metadata") { // NOLINT(clang-analyzer-optin.portability.UnixAPI) std::string value; - if (!getValue({"node", "metadata", "wasm_node_get_key"}, &value)) { + if (!getValue({"node", "metadata", "wasm_node_get_key"}, &value)) { // NOLINT(clang-analyzer-optin.portability.UnixAPI) logDebug("missing node metadata"); } logDebug(std::string("onTick ") + value); diff --git a/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc b/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc index 7a53fcba77e4..407bd5d7782b 100644 --- a/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc +++ b/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc @@ -97,11 +97,11 @@ class CommandLookUpSpeedTest { } // namespace Extensions } // namespace Envoy -static void BM_MakeRequests(benchmark::State& state) { +static void bmMakeRequests(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandLookUpSpeedTest context; - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) context.makeRequests(); } } -BENCHMARK(BM_MakeRequests); +BENCHMARK(bmMakeRequests); diff --git a/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc b/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc index 0feefb51b924..14c616d83ee8 100644 --- a/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc +++ b/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc @@ -88,44 +88,44 @@ class CommandSplitSpeedTest { } // namespace Extensions } // namespace Envoy -static void BM_Split_CompositeArray(benchmark::State& state) { +static void bmSplitCompositeArray(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) context.createLocalCompositeArray(request); } } -BENCHMARK(BM_Split_CompositeArray)->Ranges({{1, 100}, {64, 8 << 14}}); +BENCHMARK(bmSplitCompositeArray)->Ranges({{1, 100}, {64, 8 << 14}}); -static void BM_Split_Copy(benchmark::State& state) { +static void bmSplitCopy(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) context.copy(request); } } -BENCHMARK(BM_Split_Copy)->Ranges({{1, 100}, {64, 8 << 14}}); +BENCHMARK(bmSplitCopy)->Ranges({{1, 100}, {64, 8 << 14}}); -static void BM_Split_CreateShared(benchmark::State& state) { +static void bmSplitCreateShared(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) context.createShared(request); } state.counters["use_count"] = request.use_count(); } -BENCHMARK(BM_Split_CreateShared)->Ranges({{1, 100}, {64, 8 << 14}}); +BENCHMARK(bmSplitCreateShared)->Ranges({{1, 100}, {64, 8 << 14}}); -static void BM_Split_CreateVariant(benchmark::State& state) { +static void bmSplitCreateVariant(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) context.createVariant(request); } state.counters["use_count"] = request.use_count(); } -BENCHMARK(BM_Split_CreateVariant)->Ranges({{1, 100}, {64, 8 << 14}}); +BENCHMARK(bmSplitCreateVariant)->Ranges({{1, 100}, {64, 8 << 14}}); diff --git a/test/extensions/quic/connection_id_generator/envoy_deterministic_connection_id_generator_test.cc b/test/extensions/quic/connection_id_generator/envoy_deterministic_connection_id_generator_test.cc index 50366bdd4c96..023512ffb18e 100644 --- a/test/extensions/quic/connection_id_generator/envoy_deterministic_connection_id_generator_test.cc +++ b/test/extensions/quic/connection_id_generator/envoy_deterministic_connection_id_generator_test.cc @@ -20,11 +20,10 @@ using ::quic::test::TestConnectionIdNineBytesLong; class EnvoyDeterministicConnectionIdGeneratorTest : public QuicTest { public: EnvoyDeterministicConnectionIdGeneratorTest() - : connection_id_length_(12), - generator_(EnvoyDeterministicConnectionIdGenerator(connection_id_length_)) {} + : generator_(EnvoyDeterministicConnectionIdGenerator(connection_id_length_)) {} protected: - int connection_id_length_ = 0; + int connection_id_length_{12}; EnvoyDeterministicConnectionIdGenerator generator_; }; diff --git a/test/extensions/tracers/datadog/span_test.cc b/test/extensions/tracers/datadog/span_test.cc index b55f3ab2d07c..7d7089fded10 100644 --- a/test/extensions/tracers/datadog/span_test.cc +++ b/test/extensions/tracers/datadog/span_test.cc @@ -84,8 +84,7 @@ class MockIDGenerator : public datadog::tracing::IDGenerator { class DatadogTracerSpanTest : public testing::Test { public: DatadogTracerSpanTest() - : id_(0xcafebabe), collector_(std::make_shared()), - config_(makeConfig(collector_)), + : collector_(std::make_shared()), config_(makeConfig(collector_)), tracer_( // Override the tracer's ID generator so that all trace IDs and span // IDs are 0xcafebabe. @@ -108,7 +107,7 @@ class DatadogTracerSpanTest : public testing::Test { } protected: - const std::uint64_t id_ = 0; + const std::uint64_t id_{0xcafebabe}; const std::shared_ptr collector_; const datadog::tracing::TracerConfig config_; datadog::tracing::Tracer tracer_; diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 14fec9379f89..073095511aa2 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -125,9 +125,7 @@ class TestUtilOptions : public TestUtilOptionsBase { TestUtilOptions(const std::string& client_ctx_yaml, const std::string& server_ctx_yaml, bool expect_success, Network::Address::IpVersion version) : TestUtilOptionsBase(expect_success, version), client_ctx_yaml_(client_ctx_yaml), - server_ctx_yaml_(server_ctx_yaml), expect_no_cert_(false), expect_no_cert_chain_(false), - expect_private_key_method_(false), - expected_server_close_event_(Network::ConnectionEvent::RemoteClose) { + server_ctx_yaml_(server_ctx_yaml) { if (expect_success) { setExpectedServerStats("ssl.handshake"); } else { @@ -305,11 +303,11 @@ class TestUtilOptions : public TestUtilOptionsBase { const std::string client_ctx_yaml_; const std::string server_ctx_yaml_; - bool expect_no_cert_; - bool expect_no_cert_chain_; - bool expect_private_key_method_; + bool expect_no_cert_{false}; + bool expect_no_cert_chain_{false}; + bool expect_private_key_method_{false}; NiceMock runtime_; - Network::ConnectionEvent expected_server_close_event_; + Network::ConnectionEvent expected_server_close_event_{Network::ConnectionEvent::RemoteClose}; std::string expected_sha256_digest_; std::string expected_sha1_digest_; std::vector expected_local_uri_; diff --git a/test/integration/upstream_http_filter_integration_test.cc b/test/integration/upstream_http_filter_integration_test.cc index 60feec21d85d..b7662f6f1278 100644 --- a/test/integration/upstream_http_filter_integration_test.cc +++ b/test/integration/upstream_http_filter_integration_test.cc @@ -489,7 +489,8 @@ class DynamicRouterOrClusterFiltersIntegrationTest public UpstreamHttpExtensionDiscoveryIntegrationTestBase { public: DynamicRouterOrClusterFiltersIntegrationTest() - : UpstreamHttpExtensionDiscoveryIntegrationTestBase(ipVersion(), std::get<1>(GetParam())) {} + : UpstreamHttpExtensionDiscoveryIntegrationTestBase( + DynamicRouterOrClusterFiltersIntegrationTest::ipVersion(), std::get<1>(GetParam())) {} Network::Address::IpVersion ipVersion() const override { return std::get<0>(std::get<0>(GetParam())); @@ -741,7 +742,8 @@ class DynamicRouterAndClusterFiltersIntegrationTest public UpstreamHttpExtensionDiscoveryIntegrationTestBase { public: DynamicRouterAndClusterFiltersIntegrationTest() - : UpstreamHttpExtensionDiscoveryIntegrationTestBase(ipVersion(), false) {} + : UpstreamHttpExtensionDiscoveryIntegrationTestBase( + DynamicRouterAndClusterFiltersIntegrationTest::ipVersion(), false) {} Network::Address::IpVersion ipVersion() const override { return std::get<0>(GetParam()); } Grpc::ClientType clientType() const override { return std::get<1>(GetParam()); } diff --git a/test/mocks/buffer/mocks.cc b/test/mocks/buffer/mocks.cc index 5ea5e9f24b8a..56fb48e0d0fe 100644 --- a/test/mocks/buffer/mocks.cc +++ b/test/mocks/buffer/mocks.cc @@ -21,6 +21,7 @@ MockBufferBase::MockBufferBase(std::function, std::fu ASSERT(0); // This constructor is not supported for OwnedImpl. } +// NOLINTNEXTLINE(modernize-use-equals-default) template <> MockBufferBase::MockBufferBase(){}; MockBufferFactory::MockBufferFactory() = default; diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index d1ee45fd9f7f..565087d6a1d7 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -618,6 +618,7 @@ bool RouterCheckTool::runtimeMock(absl::string_view key, } Options::Options(int argc, char** argv) { + // NOLINTNEXTLINE(clang-analyzer-optin.cplusplus.VirtualCall) TCLAP::CmdLine cmd("router_check_tool", ' ', "none", true); TCLAP::SwitchArg is_detailed("d", "details", "Show detailed test execution results", cmd, false); TCLAP::SwitchArg only_show_failures("", "only-show-failures", "Only display failing tests", cmd, diff --git a/test/tools/schema_validator/validator.cc b/test/tools/schema_validator/validator.cc index 60b5f5637bb9..ad5727434557 100644 --- a/test/tools/schema_validator/validator.cc +++ b/test/tools/schema_validator/validator.cc @@ -33,6 +33,7 @@ const std::string& Schema::toString(Type type) { } Options::Options(int argc, const char* const* argv) { + // NOLINTNEXTLINE(clang-analyzer-optin.cplusplus.VirtualCall) TCLAP::CmdLine cmd("schema_validator_tool", ' ', VersionInfo::version()); TCLAP::ValueArg config_path("c", "config-path", "Path to configuration file.", true, "", "string", cmd); @@ -125,7 +126,7 @@ void Validator::validate(const Options& options) { } void Validator::run(int argc, const char* const* argv) { - Options options(argc, argv); + Options options(argc, argv); // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall) Validator v; v.validate(options); diff --git a/tools/bootstrap2pb.cc b/tools/bootstrap2pb.cc index 0c41fd948efb..ef9c66e16ba0 100644 --- a/tools/bootstrap2pb.cc +++ b/tools/bootstrap2pb.cc @@ -32,6 +32,7 @@ int main(int argc, char** argv) { Envoy::Stats::IsolatedStoreImpl stats_store; Envoy::Event::RealTimeSystem time_system; // NO_CHECK_FORMAT(real_time) Envoy::Random::RandomGeneratorImpl rand; + // TODO: fix or remove - this seems to be broken Envoy::Api::Impl api(platform_impl_.threadFactory(), stats_store, time_system, platform_impl_.fileSystem(), rand); diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index ea7b558f6f5e..c41c4955937e 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -26,11 +26,13 @@ BACKTRACE BEL BBR BIDIRECTIONAL +bm BSON BPF Bdecoded Bencoded CIO +deadcode DFP DOM GiB From f4675ce7d7635a9e50deb435098139195653e0a3 Mon Sep 17 00:00:00 2001 From: Ryan Northey Date: Fri, 22 Sep 2023 17:31:17 +0100 Subject: [PATCH 2/4] assisted-fixes Signed-off-by: Ryan Northey --- test/common/common/utility_speed_test.cc | 48 ++++++++++++------- .../grpc/async_client_manager_benchmark.cc | 6 ++- test/common/http/codes_speed_test.cc | 6 ++- .../common/network/address_impl_speed_test.cc | 6 ++- test/common/network/lc_trie_speed_test.cc | 18 ++++--- .../redis_proxy/command_lookup_speed_test.cc | 3 +- .../redis_proxy/command_split_speed_test.cc | 12 +++-- 7 files changed, 66 insertions(+), 33 deletions(-) diff --git a/test/common/common/utility_speed_test.cc b/test/common/common/utility_speed_test.cc index 1fbfb91fcd96..153db7dc846b 100644 --- a/test/common/common/utility_speed_test.cc +++ b/test/common/common/utility_speed_test.cc @@ -32,12 +32,13 @@ static void bmAccessLogDateTimeFormatter(benchmark::State& state) { static Envoy::SystemTime time(std::chrono::seconds(1522796769)); static std::mt19937 prng(1); // PRNG with a fixed seed, for repeatability static std::uniform_int_distribution distribution(-10, 20); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { // TODO(brian-pane): The next line, which computes the next input timestamp, // currently accounts for ~30% of the CPU time of this benchmark test. If // the AccessLogDateTimeFormatter implementation is optimized further, we // should precompute a sequence of input timestamps so the benchmark's own // overhead won't obscure changes in the speed of the code being benchmarked. + UNREFERENCED_PARAMETER(_); time += std::chrono::milliseconds(static_cast(distribution(prng))); outputBytes += Envoy::AccessLogDateTimeFormatter::fromTime(time).length(); } @@ -54,7 +55,8 @@ static void bmDateTimeFormatterWithSubseconds(benchmark::State& state) { std::mt19937 prng(1); std::uniform_int_distribution distribution(-10, 20); Envoy::DateFormatter date_formatter("%Y-%m-%dT%H:%M:%s.%3f"); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); time += std::chrono::milliseconds(static_cast(distribution(prng))); outputBytes += date_formatter.fromTime(time).length(); } @@ -80,7 +82,8 @@ static void bmDateTimeFormatterWithLongSubsecondsString(benchmark::State& state) } absl::StrAppend(&input, duplicate_input); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); Envoy::DateFormatter date_formatter(input); time += std::chrono::milliseconds(static_cast(distribution(prng))); outputBytes += date_formatter.fromTime(time).length(); @@ -97,7 +100,8 @@ static void bmDateTimeFormatterWithoutSubseconds(benchmark::State& state) { std::mt19937 prng(1); std::uniform_int_distribution distribution(-10, 20); Envoy::DateFormatter date_formatter("%Y-%m-%dT%H:%M:%s"); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); time += std::chrono::milliseconds(static_cast(distribution(prng))); outputBytes += date_formatter.fromTime(time).length(); } @@ -107,7 +111,8 @@ BENCHMARK(bmDateTimeFormatterWithoutSubseconds); static void bmRTrimStringView(benchmark::State& state) { int accum = 0; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); absl::string_view text(TextToTrim, TextToTrimLength); text = Envoy::StringUtil::rtrim(text); accum += TextToTrimLength - text.size(); @@ -118,7 +123,8 @@ BENCHMARK(bmRTrimStringView); static void bmRTrimStringViewAlreadyTrimmed(benchmark::State& state) { int accum = 0; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); absl::string_view text(AlreadyTrimmed, AlreadyTrimmedLength); text = Envoy::StringUtil::rtrim(text); accum += AlreadyTrimmedLength - text.size(); @@ -129,7 +135,8 @@ BENCHMARK(bmRTrimStringViewAlreadyTrimmed); static void bmRTrimStringViewAlreadyTrimmedAndMakeString(benchmark::State& state) { int accum = 0; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); absl::string_view text(AlreadyTrimmed, AlreadyTrimmedLength); std::string string_copy = std::string(Envoy::StringUtil::rtrim(text)); accum += AlreadyTrimmedLength - string_copy.size(); @@ -140,7 +147,8 @@ BENCHMARK(bmRTrimStringViewAlreadyTrimmedAndMakeString); static void bmFindToken(benchmark::State& state) { const absl::string_view cache_control(CacheControl, CacheControlLength); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); RELEASE_ASSERT(Envoy::StringUtil::findToken(cache_control, ",", "no-transform"), ""); } } @@ -182,7 +190,8 @@ static bool findTokenWithoutSplitting(absl::string_view str, char delim, absl::s static void bmFindTokenWithoutSplitting(benchmark::State& state) { const absl::string_view cache_control(CacheControl, CacheControlLength); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); RELEASE_ASSERT(findTokenWithoutSplitting(cache_control, ',', "no-transform", true), ""); } } @@ -191,7 +200,8 @@ BENCHMARK(bmFindTokenWithoutSplitting); static void bmFindTokenValueNestedSplit(benchmark::State& state) { const absl::string_view cache_control(CacheControl, CacheControlLength); absl::string_view max_age; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); for (absl::string_view token : Envoy::StringUtil::splitToken(cache_control, ",")) { auto name_value = Envoy::StringUtil::splitToken(token, "="); if ((name_value.size() == 2) && (Envoy::StringUtil::trim(name_value[0]) == "max-age")) { @@ -204,7 +214,8 @@ static void bmFindTokenValueNestedSplit(benchmark::State& state) { BENCHMARK(bmFindTokenValueNestedSplit); static void bmFindTokenValueSearchForEqual(benchmark::State& state) { - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); const absl::string_view cache_control(CacheControl, CacheControlLength); absl::string_view max_age; for (absl::string_view token : Envoy::StringUtil::splitToken(cache_control, ",")) { @@ -220,7 +231,8 @@ static void bmFindTokenValueSearchForEqual(benchmark::State& state) { BENCHMARK(bmFindTokenValueSearchForEqual); static void bmFindTokenValueNoSplit(benchmark::State& state) { - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); absl::string_view cache_control(CacheControl, CacheControlLength); absl::string_view max_age; for (absl::string_view token; nextToken(cache_control, ',', true, &token);) { @@ -249,7 +261,8 @@ static void bmRemoveTokensLong(benchmark::State& state) { input.append(","); input.append(to_remove[i]); } - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); Envoy::StringUtil::removeTokens(input, ",", to_remove_set, ","); state.SetBytesProcessed(static_cast(state.iterations()) * input.size()); } @@ -257,7 +270,8 @@ static void bmRemoveTokensLong(benchmark::State& state) { BENCHMARK(bmRemoveTokensLong)->Range(8, 8 << 10); static void bmIntervalSetInsert17(benchmark::State& state) { - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); Envoy::IntervalSetImpl interval_set; interval_set.insert(7, 10); interval_set.insert(-2, -1); @@ -286,7 +300,8 @@ static void bmIntervalSet4ToVector(benchmark::State& state) { interval_set.insert(-2, -1); interval_set.insert(22, 23); interval_set.insert(8, 15); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); benchmark::DoNotOptimize(interval_set.toVector()); } } @@ -297,7 +312,8 @@ static void bmIntervalSet50ToVector(benchmark::State& state) { for (size_t i = 0; i < 100; i += 2) { interval_set.insert(i, i + 1); } - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); benchmark::DoNotOptimize(interval_set.toVector()); } } diff --git a/test/common/grpc/async_client_manager_benchmark.cc b/test/common/grpc/async_client_manager_benchmark.cc index 8498a59e8668..5770fabbccad 100644 --- a/test/common/grpc/async_client_manager_benchmark.cc +++ b/test/common/grpc/async_client_manager_benchmark.cc @@ -46,7 +46,8 @@ void testGetOrCreateAsyncClientWithConfig(::benchmark::State& state) { envoy::config::core::v3::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); for (int i = 0; i < 1000; i++) { RawAsyncClientSharedPtr foo_client0 = async_client_man_test.async_client_manager_.getOrCreateRawAsyncClient( @@ -62,7 +63,8 @@ void testGetOrCreateAsyncClientWithHashConfig(::benchmark::State& state) { grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); GrpcServiceConfigWithHashKey config_with_hash_key_a = GrpcServiceConfigWithHashKey(grpc_service); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); for (int i = 0; i < 1000; i++) { RawAsyncClientSharedPtr foo_client0 = async_client_man_test.async_client_manager_.getOrCreateRawAsyncClientWithHashKey( diff --git a/test/common/http/codes_speed_test.cc b/test/common/http/codes_speed_test.cc index 275871d86a04..06c5bb7d5e19 100644 --- a/test/common/http/codes_speed_test.cc +++ b/test/common/http/codes_speed_test.cc @@ -97,7 +97,8 @@ template class CodeUtilitySpeedTest { static void BM_AddResponsesRealSymtab(benchmark::State& state) { Envoy::Http::CodeUtilitySpeedTest context; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); context.addResponses(); } } @@ -107,7 +108,8 @@ BENCHMARK(BM_AddResponsesRealSymtab); static void BM_ResponseTimingRealSymtab(benchmark::State& state) { Envoy::Http::CodeUtilitySpeedTest context; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); context.responseTiming(); } } diff --git a/test/common/network/address_impl_speed_test.cc b/test/common/network/address_impl_speed_test.cc index fcf0a3e60cdc..f110e6adbf89 100644 --- a/test/common/network/address_impl_speed_test.cc +++ b/test/common/network/address_impl_speed_test.cc @@ -14,7 +14,8 @@ static void ipv4InstanceCreate(benchmark::State& state) { addr.sin_port = htons(443); static constexpr uint32_t Addr = 0xc00002ff; // From the RFC 5737 example range. addr.sin_addr.s_addr = htonl(Addr); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); Ipv4Instance address(&addr); benchmark::DoNotOptimize(address.ip()); } @@ -28,7 +29,8 @@ static void ipv6InstanceCreate(benchmark::State& state) { addr.sin6_port = htons(443); static const char* Addr = "2001:DB8::1234"; // From the RFC 3849 example range. inet_pton(AF_INET6, Addr, &addr.sin6_addr); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); Ipv6Instance address(addr); benchmark::DoNotOptimize(address.ip()); } diff --git a/test/common/network/lc_trie_speed_test.cc b/test/common/network/lc_trie_speed_test.cc index a245baad0383..99eb7e6aede1 100644 --- a/test/common/network/lc_trie_speed_test.cc +++ b/test/common/network/lc_trie_speed_test.cc @@ -58,7 +58,8 @@ static void lcTrieConstruct(benchmark::State& state) { CidrInputs inputs; std::unique_ptr> trie; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); trie = std::make_unique>(inputs.tag_data_); } benchmark::DoNotOptimize(trie); @@ -70,7 +71,8 @@ static void lcTrieConstructNested(benchmark::State& state) { CidrInputs inputs; std::unique_ptr> trie; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); trie = std::make_unique>( inputs.tag_data_nested_prefixes_); } @@ -83,7 +85,8 @@ static void lcTrieConstructMinimal(benchmark::State& state) { CidrInputs inputs; std::unique_ptr> trie; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) trie = std::make_unique>(inputs.tag_data_minimal_); } @@ -100,7 +103,8 @@ static void lcTrieLookup(benchmark::State& state) { static size_t i = 0; size_t output_tags = 0; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); i++; i %= address_inputs.addresses_.size(); output_tags += lc_trie->getData(address_inputs.addresses_[i]).size(); @@ -119,7 +123,8 @@ static void lcTrieLookupWithNestedPrefixes(benchmark::State& state) { static size_t i = 0; size_t output_tags = 0; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); i++; i %= address_inputs.addresses_.size(); output_tags += lc_trie_nested_prefixes->getData(address_inputs.addresses_[i]).size(); @@ -137,7 +142,8 @@ static void lcTrieLookupMinimal(benchmark::State& state) { static size_t i = 0; size_t output_tags = 0; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); i++; i %= address_inputs.addresses_.size(); output_tags += lc_trie_minimal->getData(address_inputs.addresses_[i]).size(); diff --git a/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc b/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc index 407bd5d7782b..0e7c2694328c 100644 --- a/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc +++ b/test/extensions/filters/network/redis_proxy/command_lookup_speed_test.cc @@ -100,7 +100,8 @@ class CommandLookUpSpeedTest { static void bmMakeRequests(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandLookUpSpeedTest context; - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); context.makeRequests(); } } diff --git a/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc b/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc index 14c616d83ee8..084138716546 100644 --- a/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc +++ b/test/extensions/filters/network/redis_proxy/command_split_speed_test.cc @@ -92,7 +92,8 @@ static void bmSplitCompositeArray(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); context.createLocalCompositeArray(request); } } @@ -102,7 +103,8 @@ static void bmSplitCopy(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); context.copy(request); } } @@ -112,7 +114,8 @@ static void bmSplitCreateShared(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); context.createShared(request); } state.counters["use_count"] = request.use_count(); @@ -123,7 +126,8 @@ static void bmSplitCreateVariant(benchmark::State& state) { Envoy::Extensions::NetworkFilters::RedisProxy::CommandSplitSpeedTest context; Envoy::Extensions::NetworkFilters::Common::Redis::RespValueSharedPtr request = context.makeSharedBulkStringArray(state.range(0), 36, state.range(1)); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); context.createVariant(request); } state.counters["use_count"] = request.use_count(); From 373c052d76348c4c7964e2b2765ec1e4d990f07f Mon Sep 17 00:00:00 2001 From: Ryan Northey Date: Tue, 26 Sep 2023 08:42:13 +0100 Subject: [PATCH 3/4] fixes Signed-off-by: Ryan Northey --- .clang-tidy | 5 ++++- envoy/api/io_error.h | 2 +- envoy/buffer/buffer.h | 6 ++---- envoy/common/io/io_uring.h | 2 +- envoy/router/router.h | 2 +- envoy/stats/refcount_ptr.h | 3 ++- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index d86ad55cb536..72f533b39c11 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -66,6 +66,7 @@ CheckOptions: |^evconnlistener_free$| |^event_base_free$| |^(get|set)EVP_PKEY$| + |^has_value$| |^Ip6(ntohl|htonl)$| |^get_$| |^HeaderHasValue(Ref)?$| @@ -73,12 +74,14 @@ CheckOptions: |^Is(Superset|Subset)OfHeaders$| |^LLVMFuzzerInitialize$| |^LLVMFuzzerTestOneInput$| + |^Locality$| |^MOCK_METHOD$| |^PrepareCall$| |^PrintTo$| |^resolve_dns$| |^result_type$| |Returns(Default)?WorkerId$| + |^sched_getaffinity$| |^shutdownThread_$| |TEST| |^use_count$) @@ -101,7 +104,7 @@ CheckOptions: - key: readability-identifier-naming.FunctionCase value: 'camelBack' -HeaderFilterRegex: '^./source/.*|^./contrib/.*|^./test/.*' +HeaderFilterRegex: '^./source/.*|^./contrib/.*|^./test/.*|^./envoy/.*' UseColor: true diff --git a/envoy/api/io_error.h b/envoy/api/io_error.h index ed6669bcdc8c..8e7972277ea1 100644 --- a/envoy/api/io_error.h +++ b/envoy/api/io_error.h @@ -64,7 +64,7 @@ template struct IoCallResult { : return_value_(return_value), err_(std::move(err)) {} IoCallResult(IoCallResult&& result) noexcept - : return_value_(result.return_value_), err_(std::move(result.err_)) {} + : return_value_(std::move(result.return_value_)), err_(std::move(result.err_)) {} virtual ~IoCallResult() = default; diff --git a/envoy/buffer/buffer.h b/envoy/buffer/buffer.h index b94fef17fe50..bd53cade2a8d 100644 --- a/envoy/buffer/buffer.h +++ b/envoy/buffer/buffer.h @@ -647,7 +647,7 @@ class Reservation final { // The following are for use only by implementations of Buffer. Because c++ // doesn't allow inheritance of friendship, these are just trying to make // misuse easy to spot in a code review. - static Reservation bufferImplUseOnlyConstruct(Instance& buffer) { return Reservation(buffer); } + static Reservation bufferImplUseOnlyConstruct(Instance& buffer) { return {buffer}; } decltype(slices_)& bufferImplUseOnlySlices() { return slices_; } ReservationSlicesOwnerPtr& bufferImplUseOnlySlicesOwner() { return slices_owner_; } void bufferImplUseOnlySetLength(uint64_t length) { length_ = length; } @@ -708,9 +708,7 @@ class ReservationSingleSlice final { // The following are for use only by implementations of Buffer. Because c++ // doesn't allow inheritance of friendship, these are just trying to make // misuse easy to spot in a code review. - static ReservationSingleSlice bufferImplUseOnlyConstruct(Instance& buffer) { - return ReservationSingleSlice(buffer); - } + static ReservationSingleSlice bufferImplUseOnlyConstruct(Instance& buffer) { return {buffer}; } RawSlice& bufferImplUseOnlySlice() { return slice_; } ReservationSlicesOwnerPtr& bufferImplUseOnlySliceOwner() { return slice_owner_; } }; diff --git a/envoy/common/io/io_uring.h b/envoy/common/io/io_uring.h index 602d7dede52b..a548d21157c0 100644 --- a/envoy/common/io/io_uring.h +++ b/envoy/common/io/io_uring.h @@ -260,7 +260,7 @@ using IoUringSocketPtr = std::unique_ptr; */ class IoUringWorker : public ThreadLocal::ThreadLocalObject { public: - virtual ~IoUringWorker() = default; + ~IoUringWorker() override = default; /** * Return the current thread's dispatcher. diff --git a/envoy/router/router.h b/envoy/router/router.h index a98907d64c96..73d639d640cd 100644 --- a/envoy/router/router.h +++ b/envoy/router/router.h @@ -599,7 +599,7 @@ class VirtualCluster { static VirtualClusterStats generateStats(Stats::Scope& scope, const VirtualClusterStatNames& stat_names) { - return VirtualClusterStats(stat_names, scope); + return {stat_names, scope}; } }; diff --git a/envoy/stats/refcount_ptr.h b/envoy/stats/refcount_ptr.h index b073a3a2f39a..664780b648a1 100644 --- a/envoy/stats/refcount_ptr.h +++ b/envoy/stats/refcount_ptr.h @@ -91,6 +91,7 @@ template class RefcountPtr { #endif bool operator==(const RefcountPtr& a) const { return ptr_ == a.ptr_; } bool operator!=(const RefcountPtr& a) const { return ptr_ != a.ptr_; } + // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDelete) uint32_t use_count() const { return ptr_->use_count(); } void reset() { resetInternal(); @@ -115,7 +116,7 @@ template class RefcountPtr { // forth. #if __cplusplus < 202002L template static bool operator==(std::nullptr_t, const RefcountPtr& a) { - return a == nullptr; + return a == nullptr; // NOLINT(clang-analyzer-cplusplus.Move) } template static bool operator!=(std::nullptr_t, const RefcountPtr& a) { return a != nullptr; From 138e970d155492b0421f6028f535d963a2aebbcc Mon Sep 17 00:00:00 2001 From: wbpcode Date: Wed, 27 Sep 2023 05:54:48 +0000 Subject: [PATCH 4/4] resolve clang-tidy issue of quic Signed-off-by: wbpcode Signed-off-by: Ryan Northey --- source/common/quic/envoy_quic_server_connection.h | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/source/common/quic/envoy_quic_server_connection.h b/source/common/quic/envoy_quic_server_connection.h index 5d5de00ef5c5..5c74644c880d 100644 --- a/source/common/quic/envoy_quic_server_connection.h +++ b/source/common/quic/envoy_quic_server_connection.h @@ -78,9 +78,8 @@ class QuicListenerFilterManagerImpl : public Network::QuicListenerFilterManager, } bool shouldAdvertiseServerPreferredAddress( const quic::QuicSocketAddress& server_preferred_address) const override { - // NOLINTNEXTLINE(modernize-loop-convert) - for (auto iter = accept_filters_.begin(); iter != accept_filters_.end(); iter++) { - if (!(*iter)->isCompatibleWithServerPreferredAddress(server_preferred_address)) { + for (const auto& accept_filter : accept_filters_) { + if (!accept_filter->isCompatibleWithServerPreferredAddress(server_preferred_address)) { return false; } } @@ -88,9 +87,8 @@ class QuicListenerFilterManagerImpl : public Network::QuicListenerFilterManager, } void onPeerAddressChanged(const quic::QuicSocketAddress& new_address, Network::Connection& connection) override { - // NOLINTNEXTLINE(modernize-loop-convert) - for (auto iter = accept_filters_.begin(); iter != accept_filters_.end(); iter++) { - Network::FilterStatus status = (*iter)->onPeerAddressChanged(new_address, connection); + for (auto& accept_filter : accept_filters_) { + Network::FilterStatus status = accept_filter->onPeerAddressChanged(new_address, connection); if (status == Network::FilterStatus::StopIteration || connection.state() != Network::Connection::State::Open) { return; @@ -98,9 +96,8 @@ class QuicListenerFilterManagerImpl : public Network::QuicListenerFilterManager, } } void startFilterChain() { - // NOLINTNEXTLINE(modernize-loop-convert) - for (auto iter = accept_filters_.begin(); iter != accept_filters_.end(); iter++) { - Network::FilterStatus status = (*iter)->onAccept(*this); + for (auto& accept_filter : accept_filters_) { + Network::FilterStatus status = accept_filter->onAccept(*this); if (status == Network::FilterStatus::StopIteration || !socket().ioHandle().isOpen()) { break; }