From: David Reiss Date: Tue, 21 Aug 2007 23:59:34 +0000 (+0000) Subject: Thrift: Move TStringBuffer functionality into TMemoryBuffer. X-Git-Tag: 0.2.0~1259 X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=2dc72c3864e76d9eb14c1f83def63fa2351f4fb2;p=common%2Fthrift.git Thrift: Move TStringBuffer functionality into TMemoryBuffer. Summary: TMemoryBuffer already has the necessary plubming to work with C++ strings. This revision implements that functionality with a few wrapper methods. Removed TStringBuffer as it should no longer be required (and it is tricky to safely inherit from a class that has a non-virtual destructor). Also refactored the TMemoryBuffer constructors a bit. Reviewed By: aditya, yunfang Test Plan: test/TMemoryBufferTest.cpp Revert Plan: ok git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665213 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/lib/cpp/src/transport/TTransportUtils.cpp b/lib/cpp/src/transport/TTransportUtils.cpp index 290b4835..e5523bef 100644 --- a/lib/cpp/src/transport/TTransportUtils.cpp +++ b/lib/cpp/src/transport/TTransportUtils.cpp @@ -190,7 +190,7 @@ uint32_t TMemoryBuffer::read(uint8_t* buf, uint32_t len) { return 0; } - // Device how much to give + // Decide how much to give uint32_t give = len; if (avail < len) { give = avail; @@ -203,6 +203,27 @@ uint32_t TMemoryBuffer::read(uint8_t* buf, uint32_t len) { return give; } +uint32_t TMemoryBuffer::readAppendToString(std::string& str, uint32_t len) { + // Check avaible data for reading + uint32_t avail = wPos_ - rPos_; + if (avail == 0) { + return 0; + } + + // Device how much to give + uint32_t give = len; + if (avail < len) { + give = avail; + } + + // Reserve memory, copy into string, and increment rPos_ + str.reserve(str.length()+give); + str.append((char*)buffer_ + rPos_, give); + rPos_ += give; + + return give; +} + void TMemoryBuffer::write(const uint8_t* buf, uint32_t len) { // Check available space uint32_t avail = bufferSize_ - wPos_; diff --git a/lib/cpp/src/transport/TTransportUtils.h b/lib/cpp/src/transport/TTransportUtils.h index 102e848f..e437ee99 100644 --- a/lib/cpp/src/transport/TTransportUtils.h +++ b/lib/cpp/src/transport/TTransportUtils.h @@ -8,7 +8,7 @@ #define _THRIFT_TRANSPORT_TTRANSPORTUTILS_H_ 1 #include -#include +#include #include #include @@ -242,43 +242,59 @@ class TFramedTransport : public TTransport { * @author Mark Slee */ class TMemoryBuffer : public TTransport { - public: - TMemoryBuffer() { - owner_ = true; - bufferSize_ = 1024; - buffer_ = (uint8_t*)malloc(bufferSize_); - if (buffer_ == NULL) { - throw TTransportException("Out of memory"); + private: + + // Common initialization done by all constructors. + void initCommon(uint8_t* buf, uint32_t size, bool owner, uint32_t wPos) { + if (buf == NULL) { + assert(owner); + buf = (uint8_t*)malloc(size); + if (buf == NULL) { + throw TTransportException("Out of memory"); + } } - wPos_ = 0; + + buffer_ = buf; + bufferSize_ = size; + owner_ = owner; + wPos_ = wPos; rPos_ = 0; } + public: + static const uint32_t defaultSize = 1024; + + TMemoryBuffer() { + initCommon(NULL, defaultSize, true, 0); + } + TMemoryBuffer(uint32_t sz) { - owner_ = true; - bufferSize_ = sz; - buffer_ = (uint8_t*)malloc(bufferSize_); - if (buffer_ == NULL) { - throw TTransportException("Out of memory"); - } - wPos_ = 0; - rPos_ = 0; + initCommon(NULL, sz, true, 0); } - TMemoryBuffer(uint8_t* buf, int sz) { - owner_ = false; - buffer_ = buf; - bufferSize_ = sz; - wPos_ = sz; - rPos_ = 0; + // transferOwnership should be true if you want TMemoryBuffer to call free(buf). + TMemoryBuffer(uint8_t* buf, int sz, bool transferOwnership = false) { + initCommon(buf, sz, transferOwnership, sz); + } + + // copy should be true if you want TMemoryBuffer to make a copy of the string. + // If you set copy to false, the string must not be destroyed before you are + // done with the TMemoryBuffer. + TMemoryBuffer(const std::string& str, bool copy = false) { + if (copy) { + initCommon(NULL, str.length(), true, 0); + this->write(reinterpret_cast(str.data()), str.length()); + } else { + // This first argument should be equivalent to the following: + // const_cast(reinterpret_cast(str.data())) + initCommon((uint8_t*)str.data(), str.length(), false, str.length()); + } } ~TMemoryBuffer() { if (owner_) { - if (buffer_ != NULL) { - free(buffer_); - buffer_ = NULL; - } + free(buffer_); + buffer_ = NULL; } } @@ -299,26 +315,56 @@ class TMemoryBuffer : public TTransport { *sz = wPos_; } + std::string getBufferAsString() { + return std::string((char*)buffer_, (std::string::size_type)bufferSize_); + } + + void appendBufferToString(std::string& str) { + str.append((char*)buffer_, bufferSize_); + } + void resetBuffer() { wPos_ = 0; rPos_ = 0; } - void resetBuffer(uint8_t* buf, uint32_t sz) { + // transferOwnership should be true if you want TMemoryBuffer to call free(buf). + void resetBuffer(uint8_t* buf, uint32_t sz, bool transferOwnership = false) { if (owner_) { - if (buffer_ != NULL) { - free(buffer_); - } + free(buffer_); } - owner_ = false; + owner_ = donate; buffer_ = buf; bufferSize_ = sz; wPos_ = sz; rPos_ = 0; } + // See the constructor that takes a string. + void resetFromString(const std::string& str, bool copy = false) { + if (copy) { + uint8_t* buf = (uint8_t*)malloc(str.length()); + if (buf == NULL) { + throw TTransportException("Out of memory"); + } + std::copy(str.begin(), str.end(), buf); + resetBuffer(buf, str.length(), true); + } else { + // See the above comment about const_cast. + resetBuffer((uint8_t*)str.data(), str.length(), false); + } + } + uint32_t read(uint8_t* buf, uint32_t len); + std::string readAsString(uint32_t len) { + std::string str; + (void)readAppendToString(str, len); + return str; + } + + uint32_t readAppendToString(std::string& str, uint32_t len); + void readEnd() { if (rPos_ == wPos_) { resetBuffer(); @@ -331,82 +377,24 @@ class TMemoryBuffer : public TTransport { return wPos_ - rPos_; } - protected: + private: // Data buffer uint8_t* buffer_; // Allocated buffer size uint32_t bufferSize_; - // Is this object the owner of the buffer? - bool owner_; - - private: // Where the write is at uint32_t wPos_; // Where the reader is at uint32_t rPos_; + // Is this object the owner of the buffer? + bool owner_; }; -/** - * A string buffer is a tranpsort that simply reads from and writes to a - * string. Anytime you call write on it, the data is serialized - * into the underlying buffer, you can call getString() to get the serialized - * string. Before you call read, you should call resetString(data) to set the - * underlying buffer, you can then call read to get the - * de-serialized data structure. - * - * The string buffer is inherited from the memory buffer - * Thus, buffers are allocated using C constructs malloc,realloc, and the size - * doubles as necessary. - * - * @author Yun-Fang Juan - */ - -class TStringBuffer : public TMemoryBuffer { - public: - TStringBuffer() : TMemoryBuffer() {}; - - TStringBuffer(const std::string& inputString) : TMemoryBuffer() { - resetString(inputString); - }; - - //destructor. basically the same as TMemoryBuffer - ~TStringBuffer() { - if (owner_) { - if (buffer_ != NULL) { - free(buffer_); - buffer_ = NULL; - } - } - } - - //from buffer to string - std::string getString() { - std::stringstream ss; - ss.write((char*)buffer_, bufferSize_); - return ss.str(); - }; - - //from string to buffer - void resetString(const std::string& inputString) { - char* data; - std::stringstream ss; - bufferSize_ = inputString.size(); - - ss.str(inputString); - data = (char*)malloc(bufferSize_); - ss.read(data, bufferSize_); - - resetBuffer((uint8_t*)data, bufferSize_); - //set the owner_ to true so the buffer_ will be freed later on - owner_ = true; - }; -}; - /** * TPipedTransport. This transport allows piping of a request from one * transport to another either when readEnd() or writeEnd(). The typicAL diff --git a/test/DebugProtoTest.cpp b/test/DebugProtoTest.cpp index 6c773a1f..bae2a162 100644 --- a/test/DebugProtoTest.cpp +++ b/test/DebugProtoTest.cpp @@ -1,6 +1,6 @@ /* thrift -cpp DebugProtoTest.thrift -g++ -Wall -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ +g++ -Wall -g -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ DebugProtoTest.cpp gen-cpp/DebugProtoTest_types.cpp \ ../lib/cpp/.libs/libthrift.a -o DebugProtoTest ./DebugProtoTest diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift index 75108364..ac3b9b41 100644 --- a/test/DebugProtoTest.thrift +++ b/test/DebugProtoTest.thrift @@ -1,6 +1,6 @@ /* thrift -cpp DebugProtoTest.thrift -g++ -Wall -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ +g++ -Wall -g -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ DebugProtoTest.cpp gen-cpp/DebugProtoTest_types.cpp \ ../lib/cpp/.libs/libthrift.a -o DebugProtoTest ./DebugProtoTest diff --git a/test/OptionalRequiredTest.cpp b/test/OptionalRequiredTest.cpp index 73574ee4..643cbe95 100644 --- a/test/OptionalRequiredTest.cpp +++ b/test/OptionalRequiredTest.cpp @@ -1,6 +1,6 @@ /* ../compiler/cpp/thrift -cpp OptionalRequiredTest.thrift -g++ -Wall -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ +g++ -Wall -g -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ OptionalRequiredTest.cpp gen-cpp/OptionalRequiredTest_types.cpp \ ../lib/cpp/.libs/libthrift.a -o OptionalRequiredTest ./OptionalRequiredTest diff --git a/test/OptionalRequiredTest.thrift b/test/OptionalRequiredTest.thrift index 9738bd6a..b2652dd5 100644 --- a/test/OptionalRequiredTest.thrift +++ b/test/OptionalRequiredTest.thrift @@ -1,6 +1,6 @@ /* ../compiler/cpp/thrift -cpp OptionalRequiredTest.thrift -g++ -Wall -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ +g++ -Wall -g -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ OptionalRequiredTest.cpp gen-cpp/OptionalRequiredTest_types.cpp \ ../lib/cpp/.libs/libthrift.a -o OptionalRequiredTest ./OptionalRequiredTest diff --git a/test/TMemoryBufferTest.cpp b/test/TMemoryBufferTest.cpp new file mode 100644 index 00000000..a8975e1e --- /dev/null +++ b/test/TMemoryBufferTest.cpp @@ -0,0 +1,68 @@ +/* +thrift -cpp ThriftTest.thrift +g++ -Wall -g -I../lib/cpp/src -I/usr/local/include/boost-1_33_1 \ + TMemoryBufferTest.cpp gen-cpp/ThriftTest_types.cpp \ + ../lib/cpp/.libs/libthrift.a -o TMemoryBufferTest +./TMemoryBufferTest +*/ + +#include +#include +#include +#include +#include +#include "gen-cpp/ThriftTest_types.h" + + +int main(int argc, char** argv) { + { + using facebook::thrift::transport::TMemoryBuffer; + using facebook::thrift::protocol::TBinaryProtocol; + using boost::shared_ptr; + + shared_ptr strBuffer(new TMemoryBuffer()); + shared_ptr binaryProtcol(new TBinaryProtocol(strBuffer)); + + thrift::test::Xtruct a; + a.i32_thing = 10; + a.i64_thing = 30; + a.string_thing ="holla back a"; + + a.write(binaryProtcol.get()); + std::string serialized = strBuffer->getBufferAsString(); + + shared_ptr strBuffer2(new TMemoryBuffer()); + shared_ptr binaryProtcol2(new TBinaryProtocol(strBuffer2)); + + strBuffer2->resetFromString(serialized); + thrift::test::Xtruct a2; + a2.read(binaryProtcol2.get()); + + assert(a == a2); + } + + { + using facebook::thrift::transport::TMemoryBuffer; + using std::string; + using std::cout; + using std::endl; + + string* str1 = new string("abcd1234"); + const char* data1 = str1->data(); + TMemoryBuffer buf(*str1, true); + delete str1; + string* str2 = new string("plsreuse"); + bool obj_reuse = (str1 == str2); + bool dat_reuse = (data1 == str2->data()); + cout << "Object reuse: " << obj_reuse << " Data reuse: " << dat_reuse + << ((obj_reuse && dat_reuse) ? " YAY!" : "") << endl; + delete str2; + + string str3 = "wxyz", str4 = "6789"; + buf.readAppendToString(str3, 4); + buf.readAppendToString(str4, INT_MAX); + + assert(str3 == "wxyzabcd"); + assert(str4 == "67891234"); + } +}