Copilot commented on code in PR #3605:
URL: https://github.com/apache/brpc/pull/3605#discussion_r4266132713
##########
src/brpc/ubshm/ub_endpoint.cpp:
##########
@@ -144,6 +144,11 @@ void HelloMessage::Deserialize(void* data) {
len = butil::NetToHost64(net_len);
current_pos += sizeof(net_len);
memcpy(shm_name, current_pos, SHM_MAX_NAME_BUFF_LEN);
+ // shm_name arrives verbatim from the peer and is used downstream as a
+ // C-string (strncpy/strrchr/snprintf("%s")/shm allocate+map). A peer that
+ // fills all SHM_MAX_NAME_BUFF_LEN bytes without a terminator would make
+ // those scans run past the buffer, so force termination here.
Review Comment:
This comment explains the motivation well, but it hard-codes a list of
specific libc functions and operations. To reduce future drift, consider either
referencing the downstream call chain more abstractly (e.g., “treated as a
C-string in downstream logging and shm setup”) or pointing to the owning
function/module where the C-string usage happens. That makes the rationale
easier to keep accurate if the implementation changes.
##########
test/brpc_ubring_unittest.cpp:
##########
@@ -744,6 +744,11 @@ TEST_F(HelloMessageTest, large_len_value) {
}
TEST_F(HelloMessageTest, full_shm_name) {
+ // A peer that fills every byte of shm_name leaves no terminator on the
+ // wire. Deserialize must still hand back a valid C-string, because the
+ // handshake feeds the decoded name straight into strncpy/strrchr/
+ // snprintf("%s") and the shm allocate/map calls; without a terminator
Review Comment:
The comment is helpful but includes a raw `snprintf(\"%s\")` sequence which
can be confusing when searching/grepping for actual call sites. Consider
rephrasing to avoid quoting a format string literally (e.g., “snprintf with
%s”) and/or briefly referencing the concrete downstream helper/function names
where available. This keeps the test comment stable even if implementation
details change.
--
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]
