Copilot commented on code in PR #3702:
URL: https://github.com/apache/brpc/pull/3702#discussion_r4450609062
##########
src/brpc/redis_cluster.cpp:
##########
@@ -476,12 +476,12 @@ bool RedisClusterChannel::ExecuteMGet(const
ParsedCommand& cmd,
if (!ExecuteSingleCommand(sub_args, nullptr, &sub_result, cntl)) {
return false;
}
- values.push_back(sub_result.encoded_reply);
+ values.push_back(sub_result.encoded_reply.movable());
Review Comment:
`IOBuf::Movable` is not safe to use as a value-conversion here: its
constructor only transfers the two inline `_sv` references
(`src/butil/iobuf_inl.h:82-85`), while `append(const Movable&)` is the path
that handles a multi-reference/big view. A GET reply containing a sufficiently
large value can therefore be truncated to at most two blocks (and leave the
original big-view allocation unmanaged) when this `push_back` constructs the
vector element. Construct an empty element and move-append into it, or
otherwise use a transfer operation that preserves big views.
##########
src/brpc/redis_cluster.cpp:
##########
@@ -476,12 +476,12 @@ bool RedisClusterChannel::ExecuteMGet(const
ParsedCommand& cmd,
if (!ExecuteSingleCommand(sub_args, nullptr, &sub_result, cntl)) {
return false;
}
- values.push_back(sub_result.encoded_reply);
+ values.push_back(sub_result.encoded_reply.movable());
Review Comment:
`push_back` cannot construct the vector element directly from `Movable`: it
first materializes an `IOBuf` and then copies that temporary into the vector
because `IOBuf` has a copy constructor but no move constructor. That still
performs the block-reference increment/release this change is intended to
remove. Use `emplace_back` so the `IOBuf(const Movable&)` constructor runs in
place.
--
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]
