From: Stepan Palamarchuk <stepan@fb.com>
Date: Wed, 17 Jan 2018 11:12:41 +0000 (-0800)
Subject: Properly handle appending to the tail of the chain
X-Git-Tag: v2018.01.22.00~23
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=86cefd11d497791f00769e69fba550f710906527;p=folly.git

Properly handle appending to the tail of the chain

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
---

diff --git a/folly/io/Cursor.h b/folly/io/Cursor.h
index 838d366b..f74820dd 100644
--- a/folly/io/Cursor.h
+++ b/folly/io/Cursor.h
@@ -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();
+      }
     }
   }
 
diff --git a/folly/io/test/IOBufCursorTest.cpp b/folly/io/test/IOBufCursorTest.cpp
index 1f013da6..b9f6c50e 100644
--- a/folly/io/test/IOBufCursorTest.cpp
+++ b/folly/io/test/IOBufCursorTest.cpp
@@ -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) {