From 66b1e4da006b2f355ab81e1f8f6411b8d6748a0c Mon Sep 17 00:00:00 2001 From: Bright Chen Date: Wed, 15 Jan 2025 22:38:30 +0800 Subject: [PATCH] Fix some compilation warning issues --- .github/workflows/ci-linux.yml | 8 +++--- config_brpc.sh | 7 ++++- src/brpc/memcache.h | 2 +- src/brpc/policy/http2_rpc_protocol.cpp | 8 ++++-- src/brpc/policy/http_rpc_protocol.cpp | 32 +++++++++++------------ src/brpc/rdma/rdma_endpoint.cpp | 2 +- src/brpc/thrift_message.h | 2 +- src/bthread/task_group.cpp | 2 +- src/butil/logging.h | 36 ++++++++++++++++---------- 9 files changed, 58 insertions(+), 41 deletions(-) diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index 1854483a42..e4db5c0e1b 100644 --- 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 index 2563b0e28e..afc84a7d68 100755 --- a/config_brpc.sh +++ b/config_brpc.sh @@ -38,7 +38,7 @@ 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 @@ -46,6 +46,7 @@ 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 index 535516ed20..daedd49861 100644 --- a/src/brpc/memcache.h +++ b/src/brpc/memcache.h @@ -196,7 +196,7 @@ class MemcacheResponse : public NonreflectableMessage { 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 index 1fd93bf7bf..e202d32bc2 100644 --- 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( + _conn_ctx->local_settings().stream_window_size / + (_conn_ctx->VolatilePendingStreamSize() + 1)); // Allocate the quota of the window to each stream. - if (acc >= static_cast(_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 index 2dd18076fb..444be39708 100644 --- 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/rdma_endpoint.cpp b/src/brpc/rdma/rdma_endpoint.cpp index 4d83deacf7..4e87e9c4ba 100644 --- a/src/brpc/rdma/rdma_endpoint.cpp +++ b/src/brpc/rdma/rdma_endpoint.cpp @@ -1021,7 +1021,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(size) == g_rdma_recv_block_size) << size; } } if (DoPostRecv(_rbuf_data[_rq_received], g_rdma_recv_block_size) < 0) { diff --git a/src/brpc/thrift_message.h b/src/brpc/thrift_message.h index 881b5cf77d..e31c941968 100644 --- 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 index 170b273043..7d18a70cf9 100644 --- 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 index a26ff1e13c..58e369189b 100644 --- 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,7 +1275,7 @@ 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 @@ -1279,13 +1283,17 @@ inline std::ostream& operator<<(std::ostream& out, const std::wstring& wstr) { #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();