Skip to content

Commit 1b09f67

Browse files
committed
Fix incorrect seq id setting in thrift message begin
1 parent c24e641 commit 1b09f67

1 file changed

Lines changed: 14 additions & 13 deletions

File tree

src/brpc/policy/thrift_protocol.cpp

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ ReadThriftMessageBegin(butil::IOBuf* body,
8181
uint32_t version_and_len_buf[2];
8282
size_t k = body->copy_to(version_and_len_buf, sizeof(version_and_len_buf));
8383
if (k != sizeof(version_and_len_buf) ) {
84-
return butil::Status(-1, "Fail to copy %" PRIu64 " bytes from body",
84+
return butil::Status(-1, "Fail to copy %zu bytes from body",
8585
sizeof(version_and_len_buf));
8686
}
8787
*mtype = (apache::thrift::protocol::TMessageType)
@@ -95,7 +95,7 @@ ReadThriftMessageBegin(butil::IOBuf* body,
9595
char buf[sizeof(version_and_len_buf) + method_name_length + 4];
9696
k = body->cutn(buf, sizeof(buf));
9797
if (k != sizeof(buf)) {
98-
return butil::Status(-1, "Fail to cut %" PRIu64 " bytes", sizeof(buf));
98+
return butil::Status(-1, "Fail to cut %zu bytes", sizeof(buf));
9999
}
100100
method_name->assign(buf + sizeof(version_and_len_buf), method_name_length);
101101
// suppress strict-aliasing warning
@@ -120,7 +120,7 @@ WriteThriftMessageBegin(char* buf,
120120
p += 4;
121121
memcpy(p, method_name.data(), method_name.size());
122122
p += method_name.size();
123-
*p = htonl(seq_id);
123+
*(uint32_t*)p = htonl(seq_id);
124124
}
125125

126126
bool ReadThriftStruct(const butil::IOBuf& body,
@@ -163,6 +163,7 @@ bool ReadThriftStruct(const butil::IOBuf& body,
163163
}
164164

165165
xfer += iprot.readStructEnd();
166+
(void)xfer;
166167
iprot.getTransport()->readEnd();
167168
} catch (std::exception& e) {
168169
LOG(WARNING) << "Catched thrift exception: " << e.what();
@@ -247,7 +248,7 @@ void ThriftClosure::DoRun() {
247248
span->set_start_send_us(butil::cpuwide_time_us());
248249
}
249250
Socket* sock = accessor.get_sending_socket();
250-
MethodStatus* method_status = (server->options().thrift_service ?
251+
MethodStatus* method_status = (server->options().thrift_service ?
251252
server->options().thrift_service->_status : NULL);
252253
ConcurrencyRemover concurrency_remover(method_status, &_controller, _received_us);
253254
if (!method_status) {
@@ -321,6 +322,7 @@ void ThriftClosure::DoRun() {
321322
xfer += oprot.writeFieldEnd();
322323
xfer += oprot.writeFieldStop();
323324
xfer += oprot.writeStructEnd();
325+
(void)xfer;
324326

325327
oprot.writeMessageEnd();
326328
oprot.getTransport()->writeEnd();
@@ -343,7 +345,7 @@ void ThriftClosure::DoRun() {
343345
write_buf.append(buf, sizeof(buf));
344346
write_buf.append(_response.body.movable());
345347
}
346-
348+
347349
if (span) {
348350
span->set_response_size(write_buf.size());
349351
}
@@ -395,11 +397,9 @@ ParseResult ParseThriftMessage(butil::IOBuf* source,
395397
return MakeMessage(msg);
396398
}
397399

398-
inline void ProcessThriftFramedRequestNoExcept(ThriftService* service,
399-
Controller* cntl,
400-
ThriftFramedMessage* req,
401-
ThriftFramedMessage* res,
402-
ThriftClosure* done) {
400+
inline void ProcessThriftFramedRequestNoExcept(ThriftService* service, Controller* cntl,
401+
ThriftFramedMessage* req, ThriftFramedMessage* res,
402+
ThriftClosure* done) {
403403
// NOTE: done is not actually run before ResumeRunning() is called so that
404404
// we can still set `cntl' in the catch branch.
405405
done->SuspendRunning();
@@ -452,7 +452,7 @@ static void EndRunningCallMethodInPool(ThriftService* service,
452452
};
453453

454454
void ProcessThriftRequest(InputMessageBase* msg_base) {
455-
const int64_t start_parse_us = butil::cpuwide_time_us();
455+
const int64_t start_parse_us = butil::cpuwide_time_us();
456456

457457
DestroyingPtr<MostCommonMessage> msg(static_cast<MostCommonMessage*>(msg_base));
458458
SocketUniquePtr socket_guard(msg->ReleaseSocket());
@@ -623,7 +623,7 @@ void ProcessThriftResponse(InputMessageBase* msg_base) {
623623
}
624624

625625
// Read presult
626-
626+
627627
// MUST be ThriftFramedMessage (checked in SerializeThriftRequest)
628628
ThriftFramedMessage* response = (ThriftFramedMessage*)cntl->response();
629629
if (response) {
@@ -705,10 +705,11 @@ void SerializeThriftRequest(butil::IOBuf* request_buf, Controller* cntl,
705705

706706
// request's write
707707
xfer += req->raw_instance()->Write(&oprot);
708-
708+
709709
xfer += oprot.writeFieldEnd();
710710
xfer += oprot.writeFieldStop();
711711
xfer += oprot.writeStructEnd();
712+
(void)xfer;
712713

713714
oprot.writeMessageEnd();
714715
oprot.getTransport()->writeEnd();

0 commit comments

Comments
 (0)