Copilot commented on code in PR #3609:
URL: https://github.com/apache/brpc/pull/3609#discussion_r4278431424
##########
src/brpc/redis_command.cpp:
##########
@@ -332,7 +342,7 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt,
va_list ap) {
"formatting of conversion specifiers)";
AppendHeader(*outbuf, '*', ncomponent);
- outbuf->append(nocount_buf);
+ outbuf->append(nocount_buf.buf().movable());
Review Comment:
When `fmt` has no components (for example, `RedisCommandFormat(out, "")`),
the appender never calls `Next`, so both internal pointers remain null.
`IOBufAppender::buf()` unconditionally subtracts those pointers in `shrink()`,
making this call undefined behavior; the previous `std::string` path safely
emitted `*0\r\n`. Skip the move when `ncomponent == 0` (and add an empty-format
regression test).
This issue also appears on line 416 of the same file.
##########
src/brpc/redis_command.cpp:
##########
@@ -85,9 +85,20 @@ inline void AppendHeader(butil::IOBuf& buf, char fc,
unsigned long value) {
buf.append(header, len + 3);
}
-static void FlushComponent(std::string* out, std::string* compbuf, int* ncomp)
{
+inline void AppendHeader(butil::IOBufAppender& buf, char fc,
+ unsigned long value) {
+ char header[32];
+ header[0] = fc;
+ size_t len = AppendDecimal(header + 1, value);
+ header[len + 1] = '\r';
+ header[len + 2] = '\n';
+ buf.append(header, len + 3);
Review Comment:
`IOBufAppender::append()` returns `-1` when it cannot allocate a block, but
all three writes in this helper ignore that result. The formatter can therefore
append a partial RESP payload and still return `Status::OK()` (and the final
move does not restore the missing bytes); propagate an append failure back
through `FlushComponent` and both public formatters.
--
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]