From c4dd9cfe1893081b6783213f96d2b72b7b02d165 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Fri, 17 Nov 2017 13:39:01 -0800 Subject: [PATCH] reviewer feedback --- src/core/lib/debug/trace.cc | 16 +++++----------- src/core/lib/debug/trace.h | 19 ++++++++----------- test/core/iomgr/timer_list_test.cc | 8 ++++---- .../core/transport/connectivity_state_test.cc | 3 +-- test/core/util/tracer_peer.cc | 2 +- test/core/util/tracer_peer.h | 2 +- 6 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/core/lib/debug/trace.cc b/src/core/lib/debug/trace.cc index 04558fd91eb..4c63983bdce 100644 --- a/src/core/lib/debug/trace.cc +++ b/src/core/lib/debug/trace.cc @@ -29,13 +29,7 @@ int grpc_tracer_set_enabled(const char* name, int enabled); namespace grpc_core { -TraceFlagList* TraceFlagList::instance_ = nullptr; - -TraceFlagList* TraceFlagList::Get() { - if (instance_ == nullptr) - instance_ = static_cast(gpr_malloc(sizeof(TraceFlagList))); - return instance_; -} +TraceFlag* TraceFlagList::root_tracer_ = nullptr; bool TraceFlagList::Set(const char* name, bool enabled) { TraceFlag* t; @@ -83,7 +77,7 @@ void TraceFlagList::LogAllTracers() { // Flags register themselves on the list during construction TraceFlag::TraceFlag(bool default_enabled, const char* name) : name_(name), value_(default_enabled) { - TraceFlagList::Get()->Add(this); + TraceFlagList::Add(this); } } // namespace grpc_core @@ -121,9 +115,9 @@ static void parse(const char* s) { for (i = 0; i < nstrings; i++) { if (strings[i][0] == '-') { - grpc_core::TraceFlagList::Get()->Set(strings[i] + 1, false); + grpc_core::TraceFlagList::Set(strings[i] + 1, false); } else { - grpc_core::TraceFlagList::Get()->Set(strings[i], true); + grpc_core::TraceFlagList::Set(strings[i], true); } } @@ -144,5 +138,5 @@ void grpc_tracer_init(const char* env_var) { void grpc_tracer_shutdown(void) {} int grpc_tracer_set_enabled(const char* name, int enabled) { - return grpc_core::TraceFlagList::Get()->Set(name, enabled != 0); + return grpc_core::TraceFlagList::Set(name, enabled != 0); } diff --git a/src/core/lib/debug/trace.h b/src/core/lib/debug/trace.h index 8e3a39d173c..b58c16f3ca4 100644 --- a/src/core/lib/debug/trace.h +++ b/src/core/lib/debug/trace.h @@ -47,25 +47,19 @@ namespace grpc_core { class TraceFlag; class TraceFlagList { public: - static TraceFlagList* Get(); - bool Set(const char* name, bool enabled); - void Add(TraceFlag* flag); + static bool Set(const char* name, bool enabled); + static void Add(TraceFlag* flag); private: - static TraceFlagList* instance_; - void LogAllTracers(); - TraceFlagList() : root_tracer_(nullptr) {} - TraceFlag* root_tracer_; + static void LogAllTracers(); + static TraceFlag* root_tracer_; }; namespace testing { -void grpc_tracer_peer_enable_flag(grpc_core::TraceFlag* flag); +void grpc_tracer_enable_flag(grpc_core::TraceFlag* flag); } class TraceFlag { - friend void grpc_core::testing::grpc_tracer_peer_enable_flag(TraceFlag* flag); - friend class TraceFlagList; - public: TraceFlag(bool default_enabled, const char* name); ~TraceFlag() {} @@ -81,6 +75,9 @@ class TraceFlag { } private: + friend void grpc_core::testing::grpc_tracer_enable_flag(TraceFlag* flag); + friend class TraceFlagList; + void set_enabled(bool enabled) { #ifdef GRPC_THREADSAFE_TRACER gpr_atm_no_barrier_store(&value_, enabled); diff --git a/test/core/iomgr/timer_list_test.cc b/test/core/iomgr/timer_list_test.cc index 73be93511b9..5730a306ec1 100644 --- a/test/core/iomgr/timer_list_test.cc +++ b/test/core/iomgr/timer_list_test.cc @@ -49,8 +49,8 @@ static void add_test(void) { gpr_log(GPR_INFO, "add_test"); grpc_timer_list_init(&exec_ctx); - grpc_core::testing::grpc_tracer_peer_enable_flag(&grpc_timer_trace); - grpc_core::testing::grpc_tracer_peer_enable_flag(&grpc_timer_check_trace); + grpc_core::testing::grpc_tracer_enable_flag(&grpc_timer_trace); + grpc_core::testing::grpc_tracer_enable_flag(&grpc_timer_check_trace); memset(cb_called, 0, sizeof(cb_called)); grpc_millis start = grpc_exec_ctx_now(&exec_ctx); @@ -118,8 +118,8 @@ void destruction_test(void) { exec_ctx.now_is_valid = true; exec_ctx.now = 0; grpc_timer_list_init(&exec_ctx); - grpc_core::testing::grpc_tracer_peer_enable_flag(&grpc_timer_trace); - grpc_core::testing::grpc_tracer_peer_enable_flag(&grpc_timer_check_trace); + grpc_core::testing::grpc_tracer_enable_flag(&grpc_timer_trace); + grpc_core::testing::grpc_tracer_enable_flag(&grpc_timer_check_trace); memset(cb_called, 0, sizeof(cb_called)); grpc_timer_init( diff --git a/test/core/transport/connectivity_state_test.cc b/test/core/transport/connectivity_state_test.cc index e4d27950ecb..f33bfd2c947 100644 --- a/test/core/transport/connectivity_state_test.cc +++ b/test/core/transport/connectivity_state_test.cc @@ -137,8 +137,7 @@ static void test_subscribe_with_failure_then_destroy(void) { int main(int argc, char** argv) { grpc_test_init(argc, argv); - grpc_core::testing::grpc_tracer_peer_enable_flag( - &grpc_connectivity_state_trace); + grpc_core::testing::grpc_tracer_enable_flag(&grpc_connectivity_state_trace); test_connectivity_state_name(); test_check(); test_subscribe_then_unsubscribe(); diff --git a/test/core/util/tracer_peer.cc b/test/core/util/tracer_peer.cc index acb770515f4..34a132daa78 100644 --- a/test/core/util/tracer_peer.cc +++ b/test/core/util/tracer_peer.cc @@ -23,7 +23,7 @@ namespace grpc_core { namespace testing { -void grpc_tracer_peer_enable_flag(grpc_core::TraceFlag* flag) { +void grpc_tracer_enable_flag(grpc_core::TraceFlag* flag) { flag->set_enabled(1); } diff --git a/test/core/util/tracer_peer.h b/test/core/util/tracer_peer.h index 0b6c6dd685f..6dd04f65491 100644 --- a/test/core/util/tracer_peer.h +++ b/test/core/util/tracer_peer.h @@ -24,7 +24,7 @@ class TraceFlag; namespace testing { // enables the TraceFlag passed to it. Used for testing purposes. -void grpc_tracer_peer_enable_flag(grpc_core::TraceFlag* flag); +void grpc_tracer_enable_flag(grpc_core::TraceFlag* flag); } // namespace testing } // namespace grpc_core