Properly handle appending to the tail of the chain
authorStepan Palamarchuk <stepan@fb.com>
Wed, 17 Jan 2018 11:12:41 +0000 (03:12 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Wed, 17 Jan 2018 11:21:26 +0000 (03:21 -0800)
Summary: Currently appending to the tail of the chain would cause the cursor advancing to the beginning of the chain, which is not correct, instead we should advance to the tail.

Reviewed By: yfeldblum

Differential Revision: D6734999

fbshipit-source-id: b8b2585e0475b656f7b6bf4ed39686e2ccb2e432

folly/io/Cursor.h
folly/io/test/IOBufCursorTest.cpp

index 838d366..f74820d 100644 (file)
@@ -888,10 +888,19 @@ class RWCursor
       this->absolutePos_ += this->crtPos_ - this->crtBegin_;
       this->crtBuf_->appendChain(std::move(buf));
 
-      // Jump past the new links
-      this->crtBuf_ = nextBuf;
-      this->crtPos_ = this->crtBegin_ = this->crtBuf_->data();
-      this->crtEnd_ = this->crtBuf_->tail();
+      if (nextBuf == this->buffer_) {
+        // We've just appended to the end of the buffer, so advance to the end.
+        this->crtBuf_ = this->buffer_->prev();
+        this->crtBegin_ = this->crtBuf_->data();
+        this->crtPos_ = this->crtEnd_ = this->crtBuf_->tail();
+        // This has already been accounted for, so remove it.
+        this->absolutePos_ -= this->crtEnd_ - this->crtBegin_;
+      } else {
+        // Jump past the new links
+        this->crtBuf_ = nextBuf;
+        this->crtPos_ = this->crtBegin_ = this->crtBuf_->data();
+        this->crtEnd_ = this->crtBuf_->tail();
+      }
     }
   }
 
index 1f013da..b9f6c50 100644 (file)
@@ -426,6 +426,22 @@ TEST(IOBuf, cloneAndInsert) {
     // Check that nextBuf got set correctly
     cursor.read<uint8_t>();
   }
+  {
+    // Check that inserting at the end of the buffer keeps it at the end.
+    RWPrivateCursor cursor(iobuf1.get());
+    Cursor(iobuf1.get()).clone(cloned, 1);
+    EXPECT_EQ(1, cloned->countChainElements());
+    EXPECT_EQ(1, cloned->computeChainDataLength());
+
+    cursor.advanceToEnd();
+    EXPECT_EQ(17, cursor.getCurrentPosition());
+    cursor.insert(std::move(cloned));
+    EXPECT_EQ(18, cursor.getCurrentPosition());
+    EXPECT_EQ(0, cursor.totalLength());
+    EXPECT_EQ(12, iobuf1->countChainElements());
+    EXPECT_EQ(18, iobuf1->computeChainDataLength());
+    EXPECT_TRUE(cursor.isAtEnd());
+  }
 }
 
 TEST(IOBuf, cloneWithEmptyBufAtStart) {