From 86cefd11d497791f00769e69fba550f710906527 Mon Sep 17 00:00:00 2001 From: Stepan Palamarchuk Date: Wed, 17 Jan 2018 03:12:41 -0800 Subject: [PATCH] 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 --- folly/io/Cursor.h | 17 +++++++++++++---- folly/io/test/IOBufCursorTest.cpp | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) 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(); } + { + // 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) { -- 2.34.1