Repository: thrift Updated Branches: refs/heads/master 7fc33be18 -> 7848d887e
THRIFT-3086 fix a few minor valgrind identified issues Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/7848d887 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/7848d887 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/7848d887 Branch: refs/heads/master Commit: 7848d887e010ad0abb8a6e5857a41108ee6455b7 Parents: 7fc33be Author: Jim King <[email protected]> Authored: Mon Apr 6 21:38:06 2015 -0400 Committer: Roger Meier <[email protected]> Committed: Tue Apr 7 20:46:48 2015 +0200 ---------------------------------------------------------------------- lib/cpp/src/thrift/transport/TFileTransport.cpp | 15 +++---- lib/cpp/test/RecursiveTest.cpp | 1 + lib/cpp/test/ZlibTest.cpp | 46 ++++++++++---------- 3 files changed, 30 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/7848d887/lib/cpp/src/thrift/transport/TFileTransport.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/transport/TFileTransport.cpp b/lib/cpp/src/thrift/transport/TFileTransport.cpp index 13e4471..fe6ef9b 100644 --- a/lib/cpp/src/thrift/transport/TFileTransport.cpp +++ b/lib/cpp/src/thrift/transport/TFileTransport.cpp @@ -209,12 +209,9 @@ void TFileTransport::enqueueEvent(const uint8_t* buf, uint32_t eventLen) { return; } - eventInfo* toEnqueue = new eventInfo(); - toEnqueue->eventBuff_ = (uint8_t*)std::malloc((sizeof(uint8_t) * eventLen) + 4); - if (toEnqueue->eventBuff_ == NULL) { - delete toEnqueue; - throw std::bad_alloc(); - } + std::auto_ptr<eventInfo> toEnqueue(new eventInfo()); + toEnqueue->eventBuff_ = new uint8_t[(sizeof(uint8_t) * eventLen) + 4]; + // first 4 bytes is the event length memcpy(toEnqueue->eventBuff_, (void*)(&eventLen), 4); // actual event contents @@ -227,7 +224,6 @@ void TFileTransport::enqueueEvent(const uint8_t* buf, uint32_t eventLen) { // make sure that enqueue buffer is initialized and writer thread is running if (!bufferAndThreadInitialized_) { if (!initBufferAndWriteThread()) { - delete toEnqueue; return; } } @@ -243,8 +239,9 @@ void TFileTransport::enqueueEvent(const uint8_t* buf, uint32_t eventLen) { assert(!forceFlush_); // add to the buffer - if (!enqueueBuffer_->addEvent(toEnqueue)) { - delete toEnqueue; + eventInfo *pEvent = toEnqueue.release(); + if (!enqueueBuffer_->addEvent(pEvent)) { + delete pEvent; return; } http://git-wip-us.apache.org/repos/asf/thrift/blob/7848d887/lib/cpp/test/RecursiveTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/RecursiveTest.cpp b/lib/cpp/test/RecursiveTest.cpp index a74be91..9a7eafe 100644 --- a/lib/cpp/test/RecursiveTest.cpp +++ b/lib/cpp/test/RecursiveTest.cpp @@ -71,4 +71,5 @@ int main() { assert(false); } catch (const apache::thrift::protocol::TProtocolException& e) { } + depthLimit->nextitem.reset(); } http://git-wip-us.apache.org/repos/asf/thrift/blob/7848d887/lib/cpp/test/ZlibTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/ZlibTest.cpp b/lib/cpp/test/ZlibTest.cpp index 14b1a37..bafacf9 100644 --- a/lib/cpp/test/ZlibTest.cpp +++ b/lib/cpp/test/ZlibTest.cpp @@ -81,13 +81,13 @@ private: boost::variate_generator<boost::mt19937, boost::lognormal_distribution<double> > gen_; }; -uint8_t* gen_uniform_buffer(uint32_t buf_len, uint8_t c) { +boost::shared_array<uint8_t> gen_uniform_buffer(uint32_t buf_len, uint8_t c) { uint8_t* buf = new uint8_t[buf_len]; memset(buf, c, buf_len); - return buf; + return boost::shared_array<uint8_t>(buf); } -uint8_t* gen_compressible_buffer(uint32_t buf_len) { +boost::shared_array<uint8_t> gen_compressible_buffer(uint32_t buf_len) { uint8_t* buf = new uint8_t[buf_len]; // Generate small runs of alternately increasing and decreasing bytes @@ -116,10 +116,10 @@ uint8_t* gen_compressible_buffer(uint32_t buf_len) { step *= -1; } - return buf; + return boost::shared_array<uint8_t>(buf); } -uint8_t* gen_random_buffer(uint32_t buf_len) { +boost::shared_array<uint8_t> gen_random_buffer(uint32_t buf_len) { uint8_t* buf = new uint8_t[buf_len]; boost::uniform_smallint<uint8_t> distribution(0, UINT8_MAX); @@ -130,27 +130,27 @@ uint8_t* gen_random_buffer(uint32_t buf_len) { buf[n] = generator(); } - return buf; + return boost::shared_array<uint8_t>(buf); } /* * Test functions */ -void test_write_then_read(const uint8_t* buf, uint32_t buf_len) { +void test_write_then_read(const boost::shared_array<uint8_t> buf, uint32_t buf_len) { boost::shared_ptr<TMemoryBuffer> membuf(new TMemoryBuffer()); boost::shared_ptr<TZlibTransport> zlib_trans(new TZlibTransport(membuf)); - zlib_trans->write(buf, buf_len); + zlib_trans->write(buf.get(), buf_len); zlib_trans->finish(); boost::shared_array<uint8_t> mirror(new uint8_t[buf_len]); uint32_t got = zlib_trans->readAll(mirror.get(), buf_len); BOOST_REQUIRE_EQUAL(got, buf_len); - BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf, buf_len), 0); + BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf.get(), buf_len), 0); zlib_trans->verifyChecksum(); } -void test_separate_checksum(const uint8_t* buf, uint32_t buf_len) { +void test_separate_checksum(const boost::shared_array<uint8_t> buf, uint32_t buf_len) { // This one is tricky. I separate the last byte of the stream out // into a separate crbuf_. The last byte is part of the checksum, // so the entire read goes fine, but when I go to verify the checksum @@ -159,7 +159,7 @@ void test_separate_checksum(const uint8_t* buf, uint32_t buf_len) { // It worked. Awesome. boost::shared_ptr<TMemoryBuffer> membuf(new TMemoryBuffer()); boost::shared_ptr<TZlibTransport> zlib_trans(new TZlibTransport(membuf)); - zlib_trans->write(buf, buf_len); + zlib_trans->write(buf.get(), buf_len); zlib_trans->finish(); string tmp_buf; membuf->appendBufferToString(tmp_buf); @@ -170,16 +170,16 @@ void test_separate_checksum(const uint8_t* buf, uint32_t buf_len) { boost::shared_array<uint8_t> mirror(new uint8_t[buf_len]); uint32_t got = zlib_trans->readAll(mirror.get(), buf_len); BOOST_REQUIRE_EQUAL(got, buf_len); - BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf, buf_len), 0); + BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf.get(), buf_len), 0); zlib_trans->verifyChecksum(); } -void test_incomplete_checksum(const uint8_t* buf, uint32_t buf_len) { +void test_incomplete_checksum(const boost::shared_array<uint8_t> buf, uint32_t buf_len) { // Make sure we still get that "not complete" error if // it really isn't complete. boost::shared_ptr<TMemoryBuffer> membuf(new TMemoryBuffer()); boost::shared_ptr<TZlibTransport> zlib_trans(new TZlibTransport(membuf)); - zlib_trans->write(buf, buf_len); + zlib_trans->write(buf.get(), buf_len); zlib_trans->finish(); string tmp_buf; membuf->appendBufferToString(tmp_buf); @@ -190,7 +190,7 @@ void test_incomplete_checksum(const uint8_t* buf, uint32_t buf_len) { boost::shared_array<uint8_t> mirror(new uint8_t[buf_len]); uint32_t got = zlib_trans->readAll(mirror.get(), buf_len); BOOST_REQUIRE_EQUAL(got, buf_len); - BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf, buf_len), 0); + BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf.get(), buf_len), 0); try { zlib_trans->verifyChecksum(); BOOST_ERROR("verifyChecksum() did not report an error"); @@ -199,7 +199,7 @@ void test_incomplete_checksum(const uint8_t* buf, uint32_t buf_len) { } } -void test_read_write_mix(const uint8_t* buf, +void test_read_write_mix(const boost::shared_array<uint8_t> buf, uint32_t buf_len, const boost::shared_ptr<SizeGenerator>& write_gen, const boost::shared_ptr<SizeGenerator>& read_gen) { @@ -214,7 +214,7 @@ void test_read_write_mix(const uint8_t* buf, if (tot + write_len > buf_len) { write_len = buf_len - tot; } - zlib_trans->write(buf + tot, write_len); + zlib_trans->write(buf.get() + tot, write_len); tot += write_len; } @@ -234,15 +234,15 @@ void test_read_write_mix(const uint8_t* buf, tot += got; } - BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf, buf_len), 0); + BOOST_CHECK_EQUAL(memcmp(mirror.get(), buf.get(), buf_len), 0); zlib_trans->verifyChecksum(); } -void test_invalid_checksum(const uint8_t* buf, uint32_t buf_len) { +void test_invalid_checksum(const boost::shared_array<uint8_t> buf, uint32_t buf_len) { // Verify checksum checking. boost::shared_ptr<TMemoryBuffer> membuf(new TMemoryBuffer()); boost::shared_ptr<TZlibTransport> zlib_trans(new TZlibTransport(membuf)); - zlib_trans->write(buf, buf_len); + zlib_trans->write(buf.get(), buf_len); zlib_trans->finish(); string tmp_buf; membuf->appendBufferToString(tmp_buf); @@ -275,11 +275,11 @@ void test_invalid_checksum(const uint8_t* buf, uint32_t buf_len) { } } -void test_write_after_flush(const uint8_t* buf, uint32_t buf_len) { +void test_write_after_flush(const boost::shared_array<uint8_t> buf, uint32_t buf_len) { // write some data boost::shared_ptr<TMemoryBuffer> membuf(new TMemoryBuffer()); boost::shared_ptr<TZlibTransport> zlib_trans(new TZlibTransport(membuf)); - zlib_trans->write(buf, buf_len); + zlib_trans->write(buf.get(), buf_len); // call finish() zlib_trans->finish(); @@ -339,7 +339,7 @@ void test_no_write() { } while (0) void add_tests(boost::unit_test::test_suite* suite, - const uint8_t* buf, + const boost::shared_array<uint8_t>& buf, uint32_t buf_len, const char* name) { ADD_TEST_CASE(suite, name, test_write_then_read, buf, buf_len);
