Repository: thrift Updated Branches: refs/heads/master 9f85468eb -> 607748113
THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test This closes #486 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/60774811 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/60774811 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/60774811 Branch: refs/heads/master Commit: 6077481139933b927397c7da0088aa4678f9fb3c Parents: 9f85468 Author: Jim King <[email protected]> Authored: Sun May 10 08:08:18 2015 -0400 Committer: Roger Meier <[email protected]> Committed: Sun May 10 14:45:17 2015 +0200 ---------------------------------------------------------------------- .../src/thrift/transport/TBufferTransports.cpp | 15 +++--- lib/cpp/test/TMemoryBufferTest.cpp | 54 ++++++++++++++------ 2 files changed, 46 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/60774811/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 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) { } // 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) { http://git-wip-us.apache.org/repos/asf/thrift/blob/60774811/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 @@ BOOST_AUTO_TEST_CASE(test_roundtrip) { 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 @@ BOOST_AUTO_TEST_CASE(test_copy) { 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);
