commit 6077481139933b927397c7da0088aa4678f9fb3c
author Jim King <jim.king@simplivity.com> Sun May 10 08:08:18 2015 -0400
committer Roger Meier <roger@apache.org> Sun May 10 14:45:17 2015 +0200
tree 13f4e561465cd0ec548f46807ad79d3fe57e5fa6
parent 9f85468eb6acab173dd45a5e8d2c8a87e77923a7

THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test

This closes #486

lib/cpp/src/thrift/transport/TBufferTransports.cpp

diff --git a/lib/cpp/src/thrift/transport/TBufferTransports.cpp b/lib/cpp/src/thrift/transport/TBufferTransports.cpp
index 62737af..af551c3 100644
--- a/lib/cpp/src/thrift/transport/TBufferTransports.cpp
+++ b/lib/cpp/src/thrift/transport/TBufferTransports.cpp
@@ -369,18 +369,17 @@
   }
 
   // Allocate into a new pointer so we don't bork ours if it fails.
-  void* new_buffer = std::realloc(buffer_, new_size);
+  uint8_t* new_buffer = static_cast<uint8_t *>(std::realloc(buffer_, new_size));
   if (new_buffer == NULL) {
     throw std::bad_alloc();
   }
-  bufferSize_ = new_size;
 
-  ptrdiff_t offset = (uint8_t*)new_buffer - buffer_;
-  buffer_ += offset;
-  rBase_ += offset;
-  rBound_ += offset;
-  wBase_ += offset;
-  wBound_ = buffer_ + bufferSize_;
+  rBase_  = new_buffer + (rBase_ - buffer_) ;
+  rBound_ = new_buffer + (rBound_ - buffer_) ;
+  wBase_  = new_buffer + (wBase_ - buffer_) ;
+  wBound_ = new_buffer + new_size;
+  buffer_ = new_buffer;
+  bufferSize_ = new_size;
 }
 
 void TMemoryBuffer::writeSlow(const uint8_t* buf, uint32_t len) {

lib/cpp/test/TMemoryBufferTest.cpp

diff --git a/lib/cpp/test/TMemoryBufferTest.cpp b/lib/cpp/test/TMemoryBufferTest.cpp
index cf49477..82b9ed6 100644
--- a/lib/cpp/test/TMemoryBufferTest.cpp
+++ b/lib/cpp/test/TMemoryBufferTest.cpp
@@ -21,17 +21,48 @@
 #include <iostream>
 #include <climits>
 #include <cassert>
+#include <vector>
 #include <thrift/transport/TBufferTransports.h>
 #include <thrift/protocol/TBinaryProtocol.h>
 #include "gen-cpp/ThriftTest_types.h"
 
 BOOST_AUTO_TEST_SUITE(TMemoryBufferTest)
 
-BOOST_AUTO_TEST_CASE(test_roundtrip) {
-  using apache::thrift::transport::TMemoryBuffer;
-  using apache::thrift::protocol::TBinaryProtocol;
-  using boost::shared_ptr;
+using apache::thrift::protocol::TBinaryProtocol;
+using apache::thrift::transport::TMemoryBuffer;
+using apache::thrift::transport::TTransportException;
+using boost::shared_ptr;
+using std::cout;
+using std::endl;
+using std::string;
 
+BOOST_AUTO_TEST_CASE(test_read_write_grow)
+{
+    // Added to test the fix for THRIFT-1248
+    TMemoryBuffer uut;
+    const int maxSiz = 65536;
+    std::vector<uint8_t> buf;
+    buf.resize(maxSiz);
+    for (uint32_t i = 0; i < maxSiz; ++i)
+    {
+        buf[i] = static_cast<uint8_t>(i);
+    }
+
+    for (uint32_t i = 1; i < maxSiz; i *= 2)
+    {
+        uut.write(&buf[0], i);
+    }
+
+    for (uint32_t i = 1; i < maxSiz; i *= 2)
+    {
+        uint8_t verify[i];
+        uut.read(verify, i);
+        BOOST_CHECK_EQUAL(0, ::memcmp(verify, &buf[0], i));
+    }
+}
+
+BOOST_AUTO_TEST_CASE(test_roundtrip)
+{
   shared_ptr<TMemoryBuffer> strBuffer(new TMemoryBuffer());
   shared_ptr<TBinaryProtocol> binaryProtcol(new TBinaryProtocol(strBuffer));
 
@@ -53,12 +84,8 @@
   assert(a == a2);
 }
 
-BOOST_AUTO_TEST_CASE(test_copy) {
-  using apache::thrift::transport::TMemoryBuffer;
-  using std::string;
-  using std::cout;
-  using std::endl;
-
+BOOST_AUTO_TEST_CASE(test_copy)
+{
   string* str1 = new string("abcd1234");
   const char* data1 = str1->data();
   TMemoryBuffer buf((uint8_t*)str1->data(),
@@ -80,11 +107,8 @@
   assert(str4 == "67891234");
 }
 
-BOOST_AUTO_TEST_CASE(test_exceptions) {
-  using apache::thrift::transport::TTransportException;
-  using apache::thrift::transport::TMemoryBuffer;
-  using std::string;
-
+BOOST_AUTO_TEST_CASE(test_exceptions)
+{
   char data[] = "foo\0bar";
 
   TMemoryBuffer buf1((uint8_t*)data, 7, TMemoryBuffer::OBSERVE);