Fix some compilation warning issues (#2876)

apache · Jan 29, 2025 · 2a2f934 · 2a2f934
1 parent 56d349b
commit 2a2f934
Show file tree

Hide file tree

Showing 11 changed files with 63 additions and 48 deletions.
diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml
@@ -21,7 +21,7 @@ jobs:
     - uses: ./.github/actions/install-essential-dependences
     - uses: ./.github/actions/init-make-config
       with:
-        options: --cc=gcc --cxx=g++
+        options: --cc=gcc --cxx=g++ --werror
     - name: compile
       run: |
            make -j ${{env.proc_num}}
@@ -61,7 +61,7 @@ jobs:
     - uses: ./.github/actions/install-all-dependences
     - uses: ./.github/actions/init-make-config
       with:
-       options: --cc=gcc --cxx=g++ --with-thrift --with-glog --with-rdma  --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer
+       options: --cc=gcc --cxx=g++ --with-thrift --with-glog --with-rdma  --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer --werror
     - name: compile
       run: |
         make -j ${{env.proc_num}}
@@ -95,7 +95,7 @@ jobs:
     - uses: ./.github/actions/install-essential-dependences
     - uses: ./.github/actions/init-make-config
       with:
-        options: --cc=clang --cxx=clang++
+        options: --cc=clang --cxx=clang++ --werror
     - name: compile
       run: |
            make -j ${{env.proc_num}}
@@ -135,7 +135,7 @@ jobs:
     - uses: ./.github/actions/install-all-dependences
     - uses: ./.github/actions/init-make-config
       with:
-        options: --cc=clang --cxx=clang++ --with-thrift --with-glog --with-rdma  --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer
+        options: --cc=clang --cxx=clang++ --with-thrift --with-glog --with-rdma  --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer --werror
     - name: compile
       run: |
            make -j ${{env.proc_num}}

diff --git a/config_brpc.sh b/config_brpc.sh
@@ -38,14 +38,15 @@ else
     LDD=ldd
 fi
 
-TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx:,with-glog,with-thrift,with-rdma,with-mesalink,with-bthread-tracer,with-debug-bthread-sche-safety,with-debug-lock,nodebugsymbols -n 'config_brpc' -- "$@"`
+TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx:,with-glog,with-thrift,with-rdma,with-mesalink,with-bthread-tracer,with-debug-bthread-sche-safety,with-debug-lock,nodebugsymbols,werror -n 'config_brpc' -- "$@"`
 WITH_GLOG=0
 WITH_THRIFT=0
 WITH_RDMA=0
 WITH_MESALINK=0
 WITH_BTHREAD_TRACER=0
 BRPC_DEBUG_BTHREAD_SCHE_SAFETY=0
 DEBUGSYMBOLS=-g
+WERROR=
 BRPC_DEBUG_LOCK=0
 
 if [ $? != 0 ] ; then >&2 $ECHO "Terminating..."; exit 1 ; fi
@@ -74,6 +75,7 @@ while true; do
         --with-debug-bthread-sche-safety ) BRPC_DEBUG_BTHREAD_SCHE_SAFETY=1; shift 1 ;;
         --with-debug-lock ) BRPC_DEBUG_LOCK=1; shift 1 ;;
         --nodebugsymbols ) DEBUGSYMBOLS=; shift 1 ;;
+        --werror ) WERROR=-Werror; shift 1 ;;
         -- ) shift; break ;;
         * ) break ;;
     esac
@@ -441,6 +443,9 @@ CPPFLAGS="${CPPFLAGS} -D__const__=__unused__"
 if [ ! -z "$DEBUGSYMBOLS" ]; then
     CPPFLAGS="${CPPFLAGS} $DEBUGSYMBOLS"
 fi
+if [ ! -z "$WERROR" ]; then
+    CPPFLAGS="${CPPFLAGS} $WERROR"
+fi
 if [ "$SYSTEM" = "Darwin" ]; then
     CPPFLAGS="${CPPFLAGS} -Wno-deprecated-declarations -Wno-inconsistent-missing-override"
     version=`sw_vers -productVersion | awk -F '.' '{print $1 "." $2}'`

diff --git a/src/brpc/memcache.h b/src/brpc/memcache.h
@@ -196,7 +196,7 @@ class MemcacheResponse : public NonreflectableMessage<MemcacheResponse> {
     void Clear() override;
     bool IsInitialized() const PB_527_OVERRIDE;
 
-    size_t ByteSizeLong() const;
+    size_t ByteSizeLong() const override;
     bool MergePartialFromCodedStream(
         ::google::protobuf::io::CodedInputStream* input) PB_310_OVERRIDE;
     void SerializeWithCachedSizes(

diff --git a/src/brpc/policy/http2_rpc_protocol.cpp b/src/brpc/policy/http2_rpc_protocol.cpp
@@ -740,9 +740,13 @@ H2ParseResult H2StreamContext::OnData(
         }
     }
 
-    const int64_t acc = _deferred_window_update.fetch_add(frag_size, butil::memory_order_relaxed) + frag_size;
+    int64_t acc = frag_size +
+        _deferred_window_update.fetch_add(frag_size, butil::memory_order_relaxed);
+    int64_t quota = static_cast<int64_t>(
+        _conn_ctx->local_settings().stream_window_size /
+        (_conn_ctx->VolatilePendingStreamSize() + 1));
     // Allocate the quota of the window to each stream.
-    if (acc >= static_cast<int64_t>(_conn_ctx->local_settings().stream_window_size) / (_conn_ctx->VolatilePendingStreamSize() + 1)) {
+    if (acc >= quota) {
         if (acc > _conn_ctx->local_settings().stream_window_size) {
             LOG(ERROR) << "Fail to satisfy the stream-level flow control policy";
             return MakeH2Error(H2_FLOW_CONTROL_ERROR, frame_head.stream_id);

diff --git a/src/brpc/policy/http_rpc_protocol.cpp b/src/brpc/policy/http_rpc_protocol.cpp
@@ -1457,9 +1457,9 @@ void ProcessHttpRequest(InputMessageBase *msg) {
         return svc->CallMethod(md, cntl, NULL, NULL, done);
     }
 
-    const Server::MethodProperty* const sp =
+    const Server::MethodProperty* const mp =
         FindMethodPropertyByURI(path, server, &req_header._unresolved_path);
-    if (NULL == sp) {
+    if (NULL == mp) {
         if (security_mode) {
             std::string escape_path;
             WebEscape(path, &escape_path);
@@ -1468,36 +1468,36 @@ void ProcessHttpRequest(InputMessageBase *msg) {
             cntl->SetFailed(ENOMETHOD, "Fail to find method on `%s'", path.c_str());
         }
         return;
-    } else if (sp->service->GetDescriptor() == BadMethodService::descriptor()) {
+    } else if (mp->service->GetDescriptor() == BadMethodService::descriptor()) {
         BadMethodRequest breq;
         BadMethodResponse bres;
         butil::StringSplitter split(path.c_str(), '/');
         breq.set_service_name(std::string(split.field(), split.length()));
-        sp->service->CallMethod(sp->method, cntl, &breq, &bres, NULL);
+        mp->service->CallMethod(mp->method, cntl, &breq, &bres, NULL);
         return;
     }
     // Switch to service-specific error.
     non_service_error.release();
-    MethodStatus* method_status = sp->status;
+    MethodStatus* method_status = mp->status;
     resp_sender.set_method_status(method_status);
     if (method_status) {
         int rejected_cc = 0;
         if (!method_status->OnRequested(&rejected_cc)) {
             cntl->SetFailed(ELIMIT, "Rejected by %s's ConcurrencyLimiter, concurrency=%d",
-                            sp->method->full_name().c_str(), rejected_cc);
+                            mp->method->full_name().c_str(), rejected_cc);
             return;
         }
     }
 
     if (span) {
-        span->ResetServerSpanName(sp->method->full_name());
+        span->ResetServerSpanName(mp->method->full_name());
     }
     // NOTE: accesses to builtin services are not counted as part of
     // concurrency, therefore are not limited by ServerOptions.max_concurrency.
-    if (!sp->is_builtin_service && !sp->params.is_tabbed) {
+    if (!mp->is_builtin_service && !mp->params.is_tabbed) {
         if (socket->is_overcrowded() &&
             !server->options().ignore_eovercrowded &&
-            !sp->ignore_eovercrowded) {
+            !mp->ignore_eovercrowded) {
             cntl->SetFailed(EOVERCROWDED, "Connection to %s is overcrowded",
                             butil::endpoint2str(socket->remote_side()).c_str());
             return;
@@ -1522,8 +1522,8 @@ void ProcessHttpRequest(InputMessageBase *msg) {
         return;
     }
 
-    google::protobuf::Service* svc = sp->service;
-    const google::protobuf::MethodDescriptor* method = sp->method;
+    google::protobuf::Service* svc = mp->service;
+    const google::protobuf::MethodDescriptor* method = mp->method;
     accessor.set_method(method);
     RpcPBMessages* messages = server->options().rpc_pb_message_factory->Get(*svc, *method);;
     resp_sender.set_messages(messages);
@@ -1535,7 +1535,7 @@ void ProcessHttpRequest(InputMessageBase *msg) {
         cntl->SetFailed("Fail to new req or res");
         return;
     }
-    if (sp->params.allow_http_body_to_pb &&
+    if (mp->params.allow_http_body_to_pb &&
         method->input_type()->field_count() > 0) {
         // A protobuf service. No matter if Content-type is set to
         // applcation/json or body is empty, we have to treat body as a json
@@ -1604,10 +1604,10 @@ void ProcessHttpRequest(InputMessageBase *msg) {
                 butil::IOBufAsZeroCopyInputStream wrapper(req_body);
                 std::string err;
                 json2pb::Json2PbOptions options;
-                options.base64_to_bytes = sp->params.pb_bytes_to_base64;
-                options.array_to_single_repeated = sp->params.pb_single_repeated_to_array;
-                cntl->set_pb_bytes_to_base64(sp->params.pb_bytes_to_base64);
-                cntl->set_pb_single_repeated_to_array(sp->params.pb_single_repeated_to_array);
+                options.base64_to_bytes = mp->params.pb_bytes_to_base64;
+                options.array_to_single_repeated = mp->params.pb_single_repeated_to_array;
+                cntl->set_pb_bytes_to_base64(mp->params.pb_bytes_to_base64);
+                cntl->set_pb_single_repeated_to_array(mp->params.pb_single_repeated_to_array);
                 if (!json2pb::JsonToProtoMessage(&wrapper, req, options, &err)) {
                     cntl->SetFailed(EREQUEST, "Fail to parse http body as %s, %s",
                                     req->GetDescriptor()->full_name().c_str(), err.c_str());

diff --git a/src/brpc/rdma/block_pool.cpp b/src/brpc/rdma/block_pool.cpp
@@ -46,8 +46,8 @@ static RegisterCallback g_cb = NULL;
 static const size_t BYTES_IN_MB = 1048576;
 
 static const int BLOCK_DEFAULT = 0; // 8KB
-static const int BLOCK_LARGE = 1;  // 64KB
-static const int BLOCK_HUGE = 2;  // 2MB
+// static const int BLOCK_LARGE = 1;  // 64KB
+// static const int BLOCK_HUGE = 2;  // 2MB
 static const int BLOCK_SIZE_COUNT = 3;
 static size_t g_block_size[BLOCK_SIZE_COUNT] = { 8192, 65536, 2 * BYTES_IN_MB };
 

diff --git a/src/brpc/rdma/rdma_endpoint.cpp b/src/brpc/rdma/rdma_endpoint.cpp
@@ -60,8 +60,6 @@ DEFINE_bool(rdma_trace_verbose, false, "Print log message verbosely");
 BRPC_VALIDATE_GFLAG(rdma_trace_verbose, brpc::PassValidate);
 
 static const size_t IOBUF_BLOCK_HEADER_LEN = 32; // implementation-dependent
-static const size_t IOBUF_BLOCK_DEFAULT_PAYLOAD =
-        butil::IOBuf::DEFAULT_BLOCK_SIZE - IOBUF_BLOCK_HEADER_LEN;
 
 // DO NOT change this value unless you know the safe value!!!
 // This is the number of reserved WRs in SQ/RQ for pure ACK.
@@ -79,14 +77,14 @@ static const size_t RESERVED_WR_NUM = 3;
 static const char* MAGIC_STR = "RDMA";
 static const size_t MAGIC_STR_LEN = 4;
 static const size_t HELLO_MSG_LEN_MIN = 40;
-static const size_t HELLO_MSG_LEN_MAX = 4096;
+// static const size_t HELLO_MSG_LEN_MAX = 4096;
 static const size_t ACK_MSG_LEN = 4;
 static uint16_t g_rdma_hello_msg_len = 40;  // In Byte
 static uint16_t g_rdma_hello_version = 2;
 static uint16_t g_rdma_impl_version = 1;
 static uint32_t g_rdma_recv_block_size = 0;
 
-static const uint32_t MAX_INLINE_DATA = 64;
+// static const uint32_t MAX_INLINE_DATA = 64;
 static const uint8_t MAX_HOP_LIMIT = 16;
 static const uint8_t TIMEOUT = 14;
 static const uint8_t RETRY_CNT = 7;
@@ -1021,7 +1019,7 @@ int RdmaEndpoint::PostRecv(uint32_t num, bool zerocopy) {
                 PLOG(WARNING) << "Fail to allocate rbuf";
                 return -1;
             } else {
-                CHECK(size == g_rdma_recv_block_size) << size;
+                CHECK(static_cast<uint32_t>(size) == g_rdma_recv_block_size) << size;
             }
         }
         if (DoPostRecv(_rbuf_data[_rq_received], g_rdma_recv_block_size) < 0) {

diff --git a/src/brpc/rdma/rdma_helper.cpp b/src/brpc/rdma/rdma_helper.cpp
@@ -94,7 +94,7 @@ DEFINE_string(rdma_device, "", "The name of the HCA device used "
 DEFINE_int32(rdma_port, 1, "The port number to use. For RoCE, it is always 1.");
 DEFINE_int32(rdma_gid_index, -1, "The GID index to use. -1 means using the last one.");
 
-static const size_t SYSFS_SIZE = 4096;
+// static const size_t SYSFS_SIZE = 4096;
 static ibv_device** g_devices = NULL;
 static ibv_context* g_context = NULL;
 static SocketId g_async_socket;

diff --git a/src/brpc/thrift_message.h b/src/brpc/thrift_message.h
@@ -81,7 +81,7 @@ friend class ThriftStub;
     void Swap(ThriftFramedMessage* other);
 
     // implements Message ----------------------------------------------
-    void MergeFrom(const ThriftFramedMessage& from);
+    void MergeFrom(const ThriftFramedMessage& from) override;
     void Clear() override;
 
     size_t ByteSizeLong() const override;

diff --git a/src/bthread/task_group.cpp b/src/bthread/task_group.cpp
@@ -999,7 +999,7 @@ void print_task(std::ostream& os, bthread_t tid) {
     bthread_attr_t attr = BTHREAD_ATTR_NORMAL;
     bool has_tls = false;
     int64_t cpuwide_start_ns = 0;
-    TaskStatistics stat = {0, 0};
+    TaskStatistics stat = {0, 0, 0};
     TaskStatus status = TASK_STATUS_UNKNOWN;
     bool traced = false;
     pid_t worker_tid = 0;

diff --git a/src/butil/logging.h b/src/butil/logging.h
@@ -53,23 +53,23 @@
 #  define DPLOG_IF(...) DLOG_IF(__VA_ARGS__)
 #  define DPCHECK(...) DCHECK(__VA_ARGS__)
 #  define DVPLOG(...) DVLOG(__VA_ARGS__)
-# endif
+# endif // DCHECK_IS_ON()
 
 #ifndef LOG_BACKTRACE_IF
 #define LOG_BACKTRACE_IF(severity, condition) LOG_IF(severity, condition)
-#endif
+#endif // LOG_BACKTRACE_IF
 
 #ifndef LOG_BACKTRACE_IF_ONCE
 #define LOG_BACKTRACE_IF_ONCE(severity, condition) LOG_IF_ONCE(severity, condition)
-#endif
+#endif // LOG_BACKTRACE_IF_ONCE
 
 #ifndef LOG_BACKTRACE_FIRST_N
 #define LOG_BACKTRACE_FIRST_N(severity, N) LOG_FIRST_N(severity, N)
-#endif
+#endif // LOG_BACKTRACE_FIRST_N
 
 #ifndef LOG_BACKTRACE_IF_FIRST_N
 #define LOG_BACKTRACE_IF_FIRST_N(severity, condition, N) LOG_IF_FIRST_N(severity, condition, N)
-#endif
+#endif // LOG_BACKTRACE_IF_FIRST_N
 
 
 #define LOG_AT(severity, file, line)                                    \
@@ -485,8 +485,10 @@ void print_vlog_sites(VLogSitePrinter*);
     BAIDU_LAZY_STREAM(LOG_STREAM(severity), LOG_IS_ON(severity))
 #define LOG_IF(severity, condition)                                     \
     BAIDU_LAZY_STREAM(LOG_STREAM(severity), LOG_IS_ON(severity) && (condition))
+#ifndef LOG_BACKTRACE_IF
 #define LOG_BACKTRACE_IF(severity, condition)                               \
     BAIDU_LAZY_STREAM(LOG_STREAM(severity).SetBacktrace(), LOG_IS_ON(severity) && (condition))
+#endif // LOG_BACKTRACE_IF
 
 // FIXME(gejun): Should always crash.
 #define LOG_ASSERT(condition)                                           \
@@ -1185,7 +1187,7 @@ inline std::ostream& operator<<(std::ostream& out, const std::wstring& wstr) {
 // Select default policy: LOG(ERROR)
 #define NOTIMPLEMENTED_POLICY 4
 #endif
-#endif
+#endif // NOTIMPLEMENTED_POLICY
 
 #if defined(COMPILER_GCC)
 // On Linux, with GCC, we can use __PRETTY_FUNCTION__ to get the demangled name
@@ -1259,9 +1261,11 @@ inline std::ostream& operator<<(std::ostream& out, const std::wstring& wstr) {
 # define LOG_ONCE(severity) LOG_FIRST_N(severity, 1)
 # define LOG_BACKTRACE_ONCE(severity) LOG_BACKTRACE_FIRST_N(severity, 1)
 # define LOG_IF_ONCE(severity, condition) LOG_IF_FIRST_N(severity, condition, 1)
+#ifndef LOG_BACKTRACE_IF_ONCE
 # define LOG_BACKTRACE_IF_ONCE(severity, condition) \
     LOG_BACKTRACE_IF_FIRST_N(severity, condition, 1)
-#endif
+#endif // LOG_BACKTRACE_IF_ONCE
+#endif // LOG_ONCE
 
 // Print a log after every N calls. First call always prints.
 // Each call to this macro has a cost of relaxed atomic increment.
@@ -1271,21 +1275,25 @@ inline std::ostream& operator<<(std::ostream& out, const std::wstring& wstr) {
      BAIDU_LOG_IF_EVERY_N_IMPL(LOG_IF, severity, true, N)
 # define LOG_IF_EVERY_N(severity, condition, N)                  \
      BAIDU_LOG_IF_EVERY_N_IMPL(LOG_IF, severity, condition, N)
-#endif
+#endif // LOG_EVERY_N
 
 // Print logs for first N calls.
 // Almost zero overhead when the log was printed for N times
 // The corresponding macro in glog is not thread-safe while this is.
 #ifndef LOG_FIRST_N
 # define LOG_FIRST_N(severity, N)                                \
      BAIDU_LOG_IF_FIRST_N_IMPL(LOG_IF, severity, true, N)
+#ifndef LOG_BACKTRACE_FIRST_N
 # define LOG_BACKTRACE_FIRST_N(severity, N)                          \
      BAIDU_LOG_IF_FIRST_N_IMPL(LOG_BACKTRACE_IF, severity, true, N)
+#endif // LOG_BACKTRACE_FIRST_N
 # define LOG_IF_FIRST_N(severity, condition, N)                  \
      BAIDU_LOG_IF_FIRST_N_IMPL(LOG_IF, severity, condition, N)
+#ifndef LOG_BACKTRACE_IF_FIRST_N
 # define LOG_BACKTRACE_IF_FIRST_N(severity, condition, N)            \
      BAIDU_LOG_IF_FIRST_N_IMPL(LOG_BACKTRACE_IF, severity, condition, N)
-#endif
+#endif // LOG_BACKTRACE_IF_FIRST_N
+#endif // LOG_FIRST_N
 
 // Print a log every second. (not present in glog). First call always prints.
 // Each call to this macro has a cost of calling gettimeofday.
@@ -1294,33 +1302,33 @@ inline std::ostream& operator<<(std::ostream& out, const std::wstring& wstr) {
      BAIDU_LOG_IF_EVERY_SECOND_IMPL(LOG_IF, severity, true)
 # define LOG_IF_EVERY_SECOND(severity, condition)                \
      BAIDU_LOG_IF_EVERY_SECOND_IMPL(LOG_IF, severity, condition)
-#endif
+#endif // LOG_EVERY_SECOND
 
 #ifndef PLOG_EVERY_N
 # define PLOG_EVERY_N(severity, N)                               \
      BAIDU_LOG_IF_EVERY_N_IMPL(PLOG_IF, severity, true, N)
 # define PLOG_IF_EVERY_N(severity, condition, N)                 \
      BAIDU_LOG_IF_EVERY_N_IMPL(PLOG_IF, severity, condition, N)
-#endif
+#endif // PLOG_EVERY_N
 
 #ifndef PLOG_FIRST_N
 # define PLOG_FIRST_N(severity, N)                               \
      BAIDU_LOG_IF_FIRST_N_IMPL(PLOG_IF, severity, true, N)
 # define PLOG_IF_FIRST_N(severity, condition, N)                 \
      BAIDU_LOG_IF_FIRST_N_IMPL(PLOG_IF, severity, condition, N)
-#endif
+#endif // PLOG_FIRST_N
 
 #ifndef PLOG_ONCE
 # define PLOG_ONCE(severity) PLOG_FIRST_N(severity, 1)
 # define PLOG_IF_ONCE(severity, condition) PLOG_IF_FIRST_N(severity, condition, 1)
-#endif
+#endif // PLOG_ONCE
 
 #ifndef PLOG_EVERY_SECOND
 # define PLOG_EVERY_SECOND(severity)                             \
      BAIDU_LOG_IF_EVERY_SECOND_IMPL(PLOG_IF, severity, true)
 # define PLOG_IF_EVERY_SECOND(severity, condition)                       \
      BAIDU_LOG_IF_EVERY_SECOND_IMPL(PLOG_IF, severity, condition)
-#endif
+#endif // PLOG_EVERY_SECOND
 
 // DEBUG_MODE is for uses like
 //   if (DEBUG_MODE) foo.CheckThatFoo();