Fix gflag namespace (#2877)

Author: chenBright

BUILD.bazel

@@ -29,7 +29,6 @@ COPTS = [
     "-D__STDC_FORMAT_MACROS",
     "-D__STDC_LIMIT_MACROS",
     "-D__STDC_CONSTANT_MACROS",
-    "-DGFLAGS_NS=google",
 ] + select({
     "//bazel/config:brpc_with_glog": ["-DBRPC_WITH_GLOG=1"],
     "//conditions:default": ["-DBRPC_WITH_GLOG=0"],

config_brpc.sh

@@ -261,15 +261,6 @@ fi
 PROTOC=$(find_bin_or_die protoc)
 
 GFLAGS_HDR=$(find_dir_of_header_or_die gflags/gflags.h)
-# namespace of gflags may not be google, grep it from source.
-GFLAGS_NS=$(grep "namespace [_A-Za-z0-9]\+ {" $GFLAGS_HDR/gflags/gflags_declare.h | head -1 | awk '{print $2}')
-if [ "$GFLAGS_NS" = "GFLAGS_NAMESPACE" ]; then
-    GFLAGS_NS=$(grep "#define GFLAGS_NAMESPACE [_A-Za-z0-9]\+" $GFLAGS_HDR/gflags/gflags_declare.h | head -1 | awk '{print $3}')
-fi
-if [ -z "$GFLAGS_NS" ]; then
-    >&2 $ECHO "Fail to grep namespace of gflags source $GFLAGS_HDR/gflags/gflags_declare.h"
-    exit 1
-fi
 
 PROTOBUF_HDR=$(find_dir_of_header_or_die google/protobuf/message.h)
 PROTOBUF_VERSION=$(grep '#define GOOGLE_PROTOBUF_VERSION [0-9]\+' $PROTOBUF_HDR/google/protobuf/stubs/common.h | awk '{print $3}')
@@ -434,7 +425,7 @@ append_to_output "STATIC_LINKINGS=$STATIC_LINKINGS"
 append_to_output "DYNAMIC_LINKINGS=$DYNAMIC_LINKINGS"
 
 # CPP means C PreProcessing, not C PlusPlus
-CPPFLAGS="${CPPFLAGS}  -DBRPC_WITH_GLOG=$WITH_GLOG -DGFLAGS_NS=$GFLAGS_NS -DBRPC_DEBUG_BTHREAD_SCHE_SAFETY=$BRPC_DEBUG_BTHREAD_SCHE_SAFETY -DBRPC_DEBUG_LOCK=$BRPC_DEBUG_LOCK"
+CPPFLAGS="${CPPFLAGS}  -DBRPC_WITH_GLOG=$WITH_GLOG -DBRPC_DEBUG_BTHREAD_SCHE_SAFETY=$BRPC_DEBUG_BTHREAD_SCHE_SAFETY -DBRPC_DEBUG_LOCK=$BRPC_DEBUG_LOCK"
 
 # Avoid over-optimizations of TLS variables by GCC>=4.8
 # See: https://github.com/apache/brpc/issues/1693

src/brpc/builtin/flags_service.cpp

@@ -63,7 +63,7 @@ static std::string HtmlReplace(const std::string& s) {
     }
 }
 
-static void PrintFlag(std::ostream& os, const GFLAGS_NS::CommandLineFlagInfo& flag,
+static void PrintFlag(std::ostream& os, const GFLAGS_NAMESPACE::CommandLineFlagInfo& flag,
                       bool use_html) {
     if (use_html) {
         os << "<tr><td>";
@@ -108,8 +108,8 @@ void FlagsService::set_value_page(Controller* cntl,
                                   ::google::protobuf::Closure* done) {
     ClosureGuard done_guard(done);
     const std::string& name = cntl->http_request().unresolved_path();
-    GFLAGS_NS::CommandLineFlagInfo info;
-    if (!GFLAGS_NS::GetCommandLineFlagInfo(name.c_str(), &info)) {
+    GFLAGS_NAMESPACE::CommandLineFlagInfo info;
+    if (!GFLAGS_NAMESPACE::GetCommandLineFlagInfo(name.c_str(), &info)) {
         cntl->SetFailed(ENOMETHOD, "No such gflag");
         return;
     }
@@ -155,8 +155,8 @@ void FlagsService::default_method(::google::protobuf::RpcController* cntl_base,
         if (use_html && cntl->http_request().uri().GetQuery("withform")) {
             return set_value_page(cntl, done_guard.release());
         }
-        GFLAGS_NS::CommandLineFlagInfo info;
-        if (!GFLAGS_NS::GetCommandLineFlagInfo(constraint.c_str(), &info)) {
+        GFLAGS_NAMESPACE::CommandLineFlagInfo info;
+        if (!GFLAGS_NAMESPACE::GetCommandLineFlagInfo(constraint.c_str(), &info)) {
             cntl->SetFailed(ENOMETHOD, "No such gflag");
             return;
         }
@@ -169,8 +169,8 @@ void FlagsService::default_method(::google::protobuf::RpcController* cntl_base,
                             constraint.c_str());
             return;
         }
-        if (GFLAGS_NS::SetCommandLineOption(constraint.c_str(),
-                                         value_str->c_str()).empty()) {
+        if (GFLAGS_NAMESPACE::SetCommandLineOption(constraint.c_str(),
+                                                   value_str->c_str()).empty()) {
             cntl->SetFailed(EPERM, "Fail to set `%s' to %s",
                             constraint.c_str(),
                             (value_str->empty() ? "empty string" : value_str->c_str()));
@@ -218,19 +218,19 @@ void FlagsService::default_method(::google::protobuf::RpcController* cntl_base,
         // Only exact names. We don't have to iterate all flags in this case.
         for (std::set<std::string>::iterator it = exact.begin();
              it != exact.end(); ++it) {
-            GFLAGS_NS::CommandLineFlagInfo info;
-            if (GFLAGS_NS::GetCommandLineFlagInfo(it->c_str(), &info)) {
+            GFLAGS_NAMESPACE::CommandLineFlagInfo info;
+            if (GFLAGS_NAMESPACE::GetCommandLineFlagInfo(it->c_str(), &info)) {
                 PrintFlag(os, info, use_html);
                 os << '\n';
             }
         }
 
     } else {
         // Iterate all flags and filter.
-        std::vector<GFLAGS_NS::CommandLineFlagInfo> flag_list;
+        std::vector<GFLAGS_NAMESPACE::CommandLineFlagInfo> flag_list;
         flag_list.reserve(128);
-        GFLAGS_NS::GetAllFlags(&flag_list);
-        for (std::vector<GFLAGS_NS::CommandLineFlagInfo>::iterator
+        GFLAGS_NAMESPACE::GetAllFlags(&flag_list);
+        for (std::vector<GFLAGS_NAMESPACE::CommandLineFlagInfo>::iterator
                  it = flag_list.begin(); it != flag_list.end(); ++it) {
             if (!constraint.empty() &&
                 exact.find(it->name) == exact.end() &&

src/brpc/builtin/rpcz_service.cpp

@@ -65,7 +65,7 @@ void RpczService::enable(::google::protobuf::RpcController* cntl_base,
     const bool use_html = UseHTML(cntl->http_request());
     cntl->http_response().set_content_type(
         use_html ? "text/html" : "text/plain");
-    if (!GFLAGS_NS::SetCommandLineOption("enable_rpcz", "true").empty()) {
+    if (!GFLAGS_NAMESPACE::SetCommandLineOption("enable_rpcz", "true").empty()) {
         if (use_html) {
             // Redirect to /rpcz
             cntl->response_attachment().append(
@@ -94,7 +94,7 @@ void RpczService::disable(::google::protobuf::RpcController* cntl_base,
     const bool use_html = UseHTML(cntl->http_request());
     cntl->http_response().set_content_type(
         use_html ? "text/html" : "text/plain");
-    if (!GFLAGS_NS::SetCommandLineOption("enable_rpcz", "false").empty()) {
+    if (!GFLAGS_NAMESPACE::SetCommandLineOption("enable_rpcz", "false").empty()) {
         if (use_html) {
             // Redirect to /rpcz
             cntl->response_attachment().append(

src/brpc/rpc_dump.cpp

@@ -139,7 +139,7 @@ void SampledRequest::destroy() {
 // Save gflags which could be reloaded at anytime.
 void RpcDumpContext::SaveFlags() {
     std::string dir;
-    CHECK(GFLAGS_NS::GetCommandLineOption("rpc_dump_dir", &dir));
+    CHECK(GFLAGS_NAMESPACE::GetCommandLineOption("rpc_dump_dir", &dir));
 
     const size_t pos = dir.find("<app>");
     if (pos != std::string::npos) {

src/bthread/bthread.cpp

@@ -24,6 +24,7 @@
 #include "butil/macros.h"                       // BAIDU_CASSERT
 #include "butil/logging.h"
 #include "butil/thread_local.h"
+#include "butil/reloadable_flags.h"
 #include "bthread/task_group.h"                // TaskGroup
 #include "bthread/task_control.h"              // TaskControl
 #include "bthread/timer_thread.h"
@@ -32,47 +33,34 @@
 
 namespace bthread {
 
+static bool validate_bthread_concurrency(const char*, int32_t val) {
+    // bthread_setconcurrency sets the flag on success path which should
+    // not be strictly in a validator. But it's OK for a int flag.
+    return bthread_setconcurrency(val) == 0;
+}
+static bool validate_bthread_min_concurrency(const char*, int32_t val);
+static bool validate_bthread_current_tag(const char*, int32_t val);
+static bool validate_bthread_concurrency_by_tag(const char*, int32_t val);
+
 DEFINE_int32(bthread_concurrency, 8 + BTHREAD_EPOLL_THREAD_NUM,
              "Number of pthread workers");
+BUTIL_VALIDATE_GFLAG(bthread_concurrency, validate_bthread_concurrency);
 
 DEFINE_int32(bthread_min_concurrency, 0,
             "Initial number of pthread workers which will be added on-demand."
             " The laziness is disabled when this value is non-positive,"
             " and workers will be created eagerly according to -bthread_concurrency and bthread_setconcurrency(). ");
+BUTIL_VALIDATE_GFLAG(bthread_min_concurrency, validate_bthread_min_concurrency);
 
 DEFINE_int32(bthread_current_tag, BTHREAD_TAG_INVALID, "Set bthread concurrency for this tag");
+BUTIL_VALIDATE_GFLAG(bthread_current_tag, validate_bthread_current_tag);
 
 DEFINE_int32(bthread_concurrency_by_tag, 8 + BTHREAD_EPOLL_THREAD_NUM,
              "Number of pthread workers of FLAGS_bthread_current_tag");
+BUTIL_VALIDATE_GFLAG(bthread_concurrency_by_tag, validate_bthread_concurrency_by_tag);
 
 static bool never_set_bthread_concurrency = true;
 
-static bool validate_bthread_concurrency(const char*, int32_t val) {
-    // bthread_setconcurrency sets the flag on success path which should
-    // not be strictly in a validator. But it's OK for a int flag.
-    return bthread_setconcurrency(val) == 0;
-}
-const int ALLOW_UNUSED register_FLAGS_bthread_concurrency = 
-    ::GFLAGS_NS::RegisterFlagValidator(&FLAGS_bthread_concurrency,
-                                    validate_bthread_concurrency);
-
-static bool validate_bthread_min_concurrency(const char*, int32_t val);
-
-const int ALLOW_UNUSED register_FLAGS_bthread_min_concurrency =
-    ::GFLAGS_NS::RegisterFlagValidator(&FLAGS_bthread_min_concurrency,
-                                    validate_bthread_min_concurrency);
-
-static bool validate_bthread_current_tag(const char*, int32_t val);
-
-const int ALLOW_UNUSED register_FLAGS_bthread_current_tag =
-    ::GFLAGS_NS::RegisterFlagValidator(&FLAGS_bthread_current_tag, validate_bthread_current_tag);
-
-static bool validate_bthread_concurrency_by_tag(const char*, int32_t val);
-
-const int ALLOW_UNUSED register_FLAGS_bthread_concurrency_by_tag =
-    ::GFLAGS_NS::RegisterFlagValidator(&FLAGS_bthread_concurrency_by_tag,
-                                       validate_bthread_concurrency_by_tag);
-
 BAIDU_CASSERT(sizeof(TaskControl*) == sizeof(butil::atomic<TaskControl*>), atomic_size_match);
 
 pthread_mutex_t g_task_control_mutex = PTHREAD_MUTEX_INITIALIZER;

src/bthread/task_group.cpp

@@ -28,6 +28,7 @@
 #include "butil/fast_rand.h"
 #include "butil/unique_ptr.h"
 #include "butil/third_party/murmurhash3/murmurhash3.h" // fmix64
+#include "butil/reloadable_flags.h"
 #include "bthread/errno.h"                  // ESTOP
 #include "bthread/butex.h"                  // butex_*
 #include "bthread/sys_futex.h"              // futex_wake_private
@@ -41,24 +42,17 @@ namespace bthread {
 static const bthread_attr_t BTHREAD_ATTR_TASKGROUP = {
     BTHREAD_STACKTYPE_UNKNOWN, 0, NULL, BTHREAD_TAG_INVALID };
 
-static bool pass_bool(const char*, bool) { return true; }
-
 DEFINE_bool(show_bthread_creation_in_vars, false, "When this flags is on, The time "
             "from bthread creation to first run will be recorded and shown in /vars");
-const bool ALLOW_UNUSED dummy_show_bthread_creation_in_vars =
-    ::GFLAGS_NS::RegisterFlagValidator(&FLAGS_show_bthread_creation_in_vars,
-                                    pass_bool);
+BUTIL_VALIDATE_GFLAG(show_bthread_creation_in_vars, butil::PassValidate);
 
 DEFINE_bool(show_per_worker_usage_in_vars, false,
             "Show per-worker usage in /vars/bthread_per_worker_usage_<tid>");
-const bool ALLOW_UNUSED dummy_show_per_worker_usage_in_vars =
-    ::GFLAGS_NS::RegisterFlagValidator(&FLAGS_show_per_worker_usage_in_vars,
-                                    pass_bool);
+BUTIL_VALIDATE_GFLAG(show_per_worker_usage_in_vars, butil::PassValidate);
 
 DEFINE_bool(bthread_enable_cpu_clock_stat, false,
             "Enable CPU clock statistics for bthread");
-const bool ALLOW_UNUSED dummy_bthread_enable_cpu_clock_stat =  ::GFLAGS_NS::RegisterFlagValidator(&FLAGS_bthread_enable_cpu_clock_stat,
-                                    pass_bool);
+BUTIL_VALIDATE_GFLAG(bthread_enable_cpu_clock_stat, butil::PassValidate);
 
 BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group, NULL);
 // Sync with TaskMeta::local_storage when a bthread is created or destroyed.

src/butil/logging.cc

@@ -102,6 +102,7 @@ typedef pthread_mutex_t* MutexHandle;
 #include "butil/containers/doubly_buffered_data.h"
 #include "butil/memory/singleton.h"
 #include "butil/endpoint.h"
+#include "butil/reloadable_flags.h"
 #ifdef BAIDU_INTERNAL
 #include "butil/comlog_sink.h"
 #endif
@@ -122,8 +123,11 @@ namespace logging {
 
 DEFINE_bool(crash_on_fatal_log, false,
             "Crash process when a FATAL log is printed");
+BUTIL_VALIDATE_GFLAG(crash_on_fatal_log, butil::PassValidate);
+
 DEFINE_bool(print_stack_on_check, true,
             "Print the stack trace when a CHECK was failed");
+BUTIL_VALIDATE_GFLAG(print_stack_on_check, butil::PassValidate);
 
 DEFINE_int32(v, 0, "Show all VLOG(m) messages for m <= this."
              " Overridable by --vmodule.");
@@ -140,6 +144,7 @@ DEFINE_bool(log_bid, true, "Log bthread id");
 DEFINE_int32(minloglevel, 0, "Any log at or above this level will be "
              "displayed. Anything below this level will be silently ignored. "
              "0=INFO 1=NOTICE 2=WARNING 3=ERROR 4=FATAL");
+BUTIL_VALIDATE_GFLAG(minloglevel, butil::NonNegativeInteger);
 
 DEFINE_bool(log_hostname, false, "Add host after pid in each log so"
             " that we know where logs came from when using aggregation tools"
@@ -1930,7 +1935,7 @@ static bool validate_vmodule(const char*, const std::string& vmodule) {
     return on_reset_vmodule(vmodule.c_str()) == 0;
 }
 
-const bool ALLOW_UNUSED validate_vmodule_dummy = GFLAGS_NS::RegisterFlagValidator(
+const bool ALLOW_UNUSED validate_vmodule_dummy = GFLAGS_NAMESPACE::RegisterFlagValidator(
     &FLAGS_vmodule, &validate_vmodule);
 
 // [Thread-safe] Reset FLAGS_v.
@@ -1959,26 +1964,7 @@ static bool validate_v(const char*, int32_t v) {
     on_reset_verbose(v);
     return true;
 }
-
-const bool ALLOW_UNUSED validate_v_dummy = GFLAGS_NS::RegisterFlagValidator(
-    &FLAGS_v, &validate_v);
-
-static bool PassValidate(const char*, bool) {
-    return true;
-}
-
-const bool ALLOW_UNUSED validate_crash_on_fatal_log =
-    GFLAGS_NS::RegisterFlagValidator(&FLAGS_crash_on_fatal_log, PassValidate);
-
-const bool ALLOW_UNUSED validate_print_stack_on_check =
-    GFLAGS_NS::RegisterFlagValidator(&FLAGS_print_stack_on_check, PassValidate);
-
-static bool NonNegativeInteger(const char*, int32_t v) {
-    return v >= 0;
-}
-
-const bool ALLOW_UNUSED validate_min_log_level = GFLAGS_NS::RegisterFlagValidator(
-    &FLAGS_minloglevel, NonNegativeInteger);
+BUTIL_VALIDATE_GFLAG(v, validate_v);
 
 }  // namespace logging
 

src/butil/reloadable_flags.h

@@ -34,12 +34,11 @@
 //
 // This macro does not work for string-flags because they're thread-unsafe to
 // modify directly. To emphasize this, you have to write the validator by
-// yourself and use GFLAGS_NS::GetCommandLineOption() to acess the flag.
-#define BUTIL_VALIDATE_GFLAG(flag, validate_fn)                     \
-    namespace butil_flags {}                                       \
-    const int register_FLAGS_ ## flag ## _dummy                    \
-                 __attribute__((__unused__)) =                     \
-        ::butil::RegisterFlagValidatorOrDieImpl<                   \
+// yourself and use GFLAGS_NAMESPACE::GetCommandLineOption() to acess the flag.
+#define BUTIL_VALIDATE_GFLAG(flag, validate_fn)                   \
+    namespace butil_flags {}                                      \
+    const int ALLOW_UNUSED register_FLAGS_ ## flag ## _dummy =    \
+        ::butil::RegisterFlagValidatorOrDieImpl<                  \
             decltype(FLAGS_##flag)>(&FLAGS_##flag, (validate_fn))
 
 
@@ -55,12 +54,17 @@ bool PositiveInteger(const char*, T v) {
     return v > 0;
 }
 
+template <typename T>
+bool NonNegativeInteger(const char*, T v) {
+    return v >= 0;
+}
+
 template <typename T>
 bool RegisterFlagValidatorOrDieImpl(
     const T* flag, bool (*validate_fn)(const char*, T val)) {
     static_assert(!butil::is_same<std::string, T>::value,
                   "Not support string flags");
-    if (GFLAGS_NS::RegisterFlagValidator(flag, validate_fn)) {
+    if (::GFLAGS_NAMESPACE::RegisterFlagValidator(flag, validate_fn)) {
         return true;
     }
     // Error printed by gflags does not have newline. Add one to it.

src/bvar/gflag.cpp

@@ -34,8 +34,8 @@ GFlag::GFlag(const butil::StringPiece& prefix,
 }
 
 void GFlag::describe(std::ostream& os, bool quote_string) const {
-    GFLAGS_NS::CommandLineFlagInfo info;
-    if (!GFLAGS_NS::GetCommandLineFlagInfo(gflag_name().c_str(), &info)) {
+    GFLAGS_NAMESPACE::CommandLineFlagInfo info;
+    if (!GFLAGS_NAMESPACE::GetCommandLineFlagInfo(gflag_name().c_str(), &info)) {
         if (quote_string) {
             os << '"';
         }
@@ -54,8 +54,8 @@ void GFlag::describe(std::ostream& os, bool quote_string) const {
 
 #ifdef BAIDU_INTERNAL
 void GFlag::get_value(boost::any* value) const {
-    GFLAGS_NS::CommandLineFlagInfo info;
-    if (!GFLAGS_NS::GetCommandLineFlagInfo(gflag_name().c_str(), &info)) {
+    GFLAGS_NAMESPACE::CommandLineFlagInfo info;
+    if (!GFLAGS_NAMESPACE::GetCommandLineFlagInfo(gflag_name().c_str(), &info)) {
         *value = "Unknown gflag=" + gflag_name();
     } else if (info.type == "string") {
         *value = info.current_value;
@@ -78,14 +78,14 @@ void GFlag::get_value(boost::any* value) const {
 
 std::string GFlag::get_value() const {
     std::string str;
-    if (!GFLAGS_NS::GetCommandLineOption(gflag_name().c_str(), &str)) {
+    if (!GFLAGS_NAMESPACE::GetCommandLineOption(gflag_name().c_str(), &str)) {
         return "Unknown gflag=" + gflag_name();
     }
     return str;
 }
 
 bool GFlag::set_value(const char* value) {
-    return !GFLAGS_NS::SetCommandLineOption(gflag_name().c_str(), value).empty();
+    return !GFLAGS_NAMESPACE::SetCommandLineOption(gflag_name().c_str(), value).empty();
 }
 
 }  // namespace bvar