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]

Reply via email to