Fix cursor insert inconsistency

Summary:
The invariant after Cursor::insert is that the cursor points to the buffer
after the insert. That invariant was not followed in the branch where the
new buffer was just prepended. This change fixes the bug.

Test Plan:
Unit test modified, all tests run with
fbconfig -r folly && fbmake runtests_opt

Reviewed By: davejwatson@fb.com

FB internal diff: D1114749

diff --git a/folly/io/Cursor.h b/folly/io/Cursor.h
index 0b79c23f26b501f9b35583163a52ac32239e7bab..5f65fb0e01a39b73507526eca628b76632767f4c 100644 (file)
--- a/folly/io/Cursor.h
+++ b/folly/io/Cursor.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 Facebook, Inc.
+ * Copyright 2014 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -501,7 +501,7 @@ class RWCursor
     folly::IOBuf* nextBuf;
     if (this->offset_ == 0) {
       // Can just prepend
-      nextBuf = buf.get();
+      nextBuf = this->crtBuf_;
       this->crtBuf_->prependChain(std::move(buf));
     } else {
       std::unique_ptr<folly::IOBuf> remaining;

diff --git a/folly/io/test/IOBufCursorTest.cpp b/folly/io/test/IOBufCursorTest.cpp
index 5cc6896792c71f2d21ba4b0c1718588678ef24e8..e0bf5a5adff69c2bf016aa9824918ff3b4d19dcf 100644 (file)
--- a/folly/io/test/IOBufCursorTest.cpp
+++ b/folly/io/test/IOBufCursorTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 Facebook, Inc.
+ * Copyright 2014 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -315,9 +315,11 @@ TEST(IOBuf, cloneAndInsert) {
     cursor.skip(1);
 
     cursor.insert(std::move(cloned));
-    EXPECT_EQ(6, iobuf1->countChainElements());
+    cursor.insert(folly::IOBuf::create(0));
+    EXPECT_EQ(7, iobuf1->countChainElements());
     EXPECT_EQ(14, iobuf1->computeChainDataLength());
-    // Check that nextBuf got set correctly
+    // Check that nextBuf got set correctly to the buffer with 1 byte left
+    EXPECT_EQ(1, cursor.peek().second);
     cursor.read<uint8_t>();
   }
 
@@ -331,7 +333,7 @@ TEST(IOBuf, cloneAndInsert) {
     cursor.skip(1);
 
     cursor.insert(std::move(cloned));
-    EXPECT_EQ(7, iobuf1->countChainElements());
+    EXPECT_EQ(8, iobuf1->countChainElements());
     EXPECT_EQ(15, iobuf1->computeChainDataLength());
     // Check that nextBuf got set correctly
     cursor.read<uint8_t>();
@@ -344,7 +346,7 @@ TEST(IOBuf, cloneAndInsert) {
     EXPECT_EQ(1, cloned->computeChainDataLength());
 
     cursor.insert(std::move(cloned));
-    EXPECT_EQ(8, iobuf1->countChainElements());
+    EXPECT_EQ(9, iobuf1->countChainElements());
     EXPECT_EQ(16, iobuf1->computeChainDataLength());
     // Check that nextBuf got set correctly
     cursor.read<uint8_t>();