From 2dc8df9ef607873ff4ad6738cb5772b624e9dde2 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Wed, 10 Mar 2021 13:38:23 -0800 Subject: [PATCH] Fix bugprone unhandled self assignment (#25667) * Add bugprone-unhandled-self-assignment * Fix bugprone-unhandled-self-assignment --- .clang-tidy | 2 -- include/grpcpp/impl/codegen/call_op_set.h | 3 +++ include/grpcpp/impl/codegen/string_ref.h | 1 + src/core/ext/filters/client_channel/lb_policy.cc | 3 +++ src/core/ext/filters/client_channel/resolver.cc | 3 +++ src/core/ext/filters/client_channel/server_address.cc | 3 +++ .../ext/filters/client_channel/subchannel_pool_interface.cc | 3 +++ src/core/lib/gprpp/ref_counted_ptr.h | 2 ++ 8 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 32662564142..8714bad3bc1 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -24,7 +24,6 @@ # - bugprone-signed-char-misuse # - bugprone-sizeof-expression # - bugprone-too-small-loop-variable -# - bugprone-unhandled-self-assignment # - clang-diagnostic-deprecated-declarations # - clang-diagnostic-unused-function # - google-readability-avoid-underscore-in-googletest-name @@ -62,7 +61,6 @@ Checks: '-*, -bugprone-signed-char-misuse, -bugprone-sizeof-expression, -bugprone-too-small-loop-variable, - -bugprone-unhandled-self-assignment, google-*, -google-readability-avoid-underscore-in-googletest-name, -google-runtime-int, diff --git a/include/grpcpp/impl/codegen/call_op_set.h b/include/grpcpp/impl/codegen/call_op_set.h index 370d51967d0..7fde1edbb92 100644 --- a/include/grpcpp/impl/codegen/call_op_set.h +++ b/include/grpcpp/impl/codegen/call_op_set.h @@ -878,6 +878,9 @@ class CallOpSet : public CallOpSetInterface, interceptor_methods_(InterceptorBatchMethodsImpl()) {} CallOpSet& operator=(const CallOpSet& other) { + if (&other == this) { + return *this; + } core_cq_tag_ = this; return_tag_ = this; call_ = other.call_; diff --git a/include/grpcpp/impl/codegen/string_ref.h b/include/grpcpp/impl/codegen/string_ref.h index 153f37102d8..4543e426042 100644 --- a/include/grpcpp/impl/codegen/string_ref.h +++ b/include/grpcpp/impl/codegen/string_ref.h @@ -51,6 +51,7 @@ class string_ref { string_ref() : data_(nullptr), length_(0) {} string_ref(const string_ref& other) : data_(other.data_), length_(other.length_) {} + // NOLINTNEXTLINE(bugprone-unhandled-self-assignment) string_ref& operator=(const string_ref& rhs) { data_ = rhs.data_; length_ = rhs.length_; diff --git a/src/core/ext/filters/client_channel/lb_policy.cc b/src/core/ext/filters/client_channel/lb_policy.cc index 4ef8cbb8b23..5470df8e121 100644 --- a/src/core/ext/filters/client_channel/lb_policy.cc +++ b/src/core/ext/filters/client_channel/lb_policy.cc @@ -70,6 +70,9 @@ LoadBalancingPolicy::UpdateArgs::UpdateArgs(UpdateArgs&& other) noexcept { LoadBalancingPolicy::UpdateArgs& LoadBalancingPolicy::UpdateArgs::operator=( const UpdateArgs& other) { + if (&other == this) { + return *this; + } addresses = other.addresses; config = other.config; grpc_channel_args_destroy(args); diff --git a/src/core/ext/filters/client_channel/resolver.cc b/src/core/ext/filters/client_channel/resolver.cc index bbcf013cc9b..a5494b1666a 100644 --- a/src/core/ext/filters/client_channel/resolver.cc +++ b/src/core/ext/filters/client_channel/resolver.cc @@ -60,6 +60,9 @@ Resolver::Result::Result(Result&& other) noexcept { } Resolver::Result& Resolver::Result::operator=(const Result& other) { + if (&other == this) { + return *this; + } addresses = other.addresses; service_config = other.service_config; GRPC_ERROR_UNREF(service_config_error); diff --git a/src/core/ext/filters/client_channel/server_address.cc b/src/core/ext/filters/client_channel/server_address.cc index 89d7c90f79c..bd8af27eb73 100644 --- a/src/core/ext/filters/client_channel/server_address.cc +++ b/src/core/ext/filters/client_channel/server_address.cc @@ -61,6 +61,9 @@ ServerAddress::ServerAddress(const ServerAddress& other) } } ServerAddress& ServerAddress::operator=(const ServerAddress& other) { + if (&other == this) { + return *this; + } address_ = other.address_; grpc_channel_args_destroy(args_); args_ = grpc_channel_args_copy(other.args_); diff --git a/src/core/ext/filters/client_channel/subchannel_pool_interface.cc b/src/core/ext/filters/client_channel/subchannel_pool_interface.cc index cdf98d209a2..308541c5309 100644 --- a/src/core/ext/filters/client_channel/subchannel_pool_interface.cc +++ b/src/core/ext/filters/client_channel/subchannel_pool_interface.cc @@ -44,6 +44,9 @@ SubchannelKey::SubchannelKey(const SubchannelKey& other) { } SubchannelKey& SubchannelKey::operator=(const SubchannelKey& other) { + if (&other == this) { + return *this; + } grpc_channel_args_destroy(const_cast(args_)); Init(other.args_, grpc_channel_args_copy); return *this; diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h index fd3bfbda87c..4ee1b950818 100644 --- a/src/core/lib/gprpp/ref_counted_ptr.h +++ b/src/core/lib/gprpp/ref_counted_ptr.h @@ -83,6 +83,7 @@ class RefCountedPtr { } // Copy assignment. + // NOLINTNEXTLINE(bugprone-unhandled-self-assignment) RefCountedPtr& operator=(const RefCountedPtr& other) { // Note: Order of reffing and unreffing is important here in case value_ // and other.value_ are the same object. @@ -235,6 +236,7 @@ class WeakRefCountedPtr { } // Copy assignment. + // NOLINTNEXTLINE(bugprone-unhandled-self-assignment) WeakRefCountedPtr& operator=(const WeakRefCountedPtr& other) { // Note: Order of reffing and unreffing is important here in case value_ // and other.value_ are the same object.