Copilot commented on code in PR #3507:
URL: https://github.com/apache/brpc/pull/3507#discussion_r3952290319
##########
src/brpc/ubshm/ub_endpoint.cpp:
##########
@@ -580,15 +672,17 @@ void* UBShmEndpoint::ProcessHandshakeAtServer(void* arg) {
uint32_t* tmp = (uint32_t*)data;
uint32_t flags = butil::NetToHost32(*tmp);
Review Comment:
Casting the handshake buffer `data` (a `uint8_t[]`) to `uint32_t*` risks
unaligned access / undefined behavior on architectures that require 32-bit
alignment. Reading the ACK flags via `memcpy` avoids alignment assumptions
while keeping the wire format unchanged.
##########
test/brpc_ubring_unittest.cpp:
##########
@@ -62,6 +62,50 @@ class HelloMessageTest : public ::testing::Test {
std::string buffer;
};
+TEST(HelloFormatExtensionTest, serialize_deserialize_roundtrip) {
+ brpc::ubring::HelloFormatExtension extension = {
+ brpc::ubring::HelloFormatExtension::WIRE_SIZE,
+ brpc::ubring::UBR_DATA_FORMAT_LEGACY_64};
+ char buffer[brpc::ubring::HelloFormatExtension::WIRE_SIZE] = {};
Review Comment:
The new format-negotiation handshake behavior (V3 extension exchange, strict
msg_len==64 handling, and fallback-to-TCP paths for NONE/unknown/mismatch) is
not covered by unit tests yet. Since this PR changes the handshake state
machine, add a small socketpair-based test (similar in spirit to
brpc_rdma_unittest.cpp) that drives client/server endpoints through: (1) V3↔V3
success selecting LEGACY_64, (2) V3↔V2 fallback without exchanging extension
bytes, and (3) V3↔V3 with server responding NONE/unknown causing ACK=0 fallback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
