From 86889762402795b78e4c1c9381d5b699e74c5fae Mon Sep 17 00:00:00 2001 From: Jim King Date: Mon, 6 Apr 2015 21:38:06 -0400 Subject: [PATCH] [THRIFT-3086] fix a few minor valgrind identified issues --- .../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(-) diff --git a/lib/cpp/src/thrift/transport/TFileTransport.cpp b/lib/cpp/src/thrift/transport/TFileTransport.cpp index 13e4471b36d..fe6ef9b8b94 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 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; } diff --git a/lib/cpp/test/RecursiveTest.cpp b/lib/cpp/test/RecursiveTest.cpp index a74be918e9d..9a7eafeedd5 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(); } diff --git a/lib/cpp/test/ZlibTest.cpp b/lib/cpp/test/ZlibTest.cpp index 14b1a373146..bafacf928ab 100644 --- a/lib/cpp/test/ZlibTest.cpp +++ b/lib/cpp/test/ZlibTest.cpp @@ -81,13 +81,13 @@ class LogNormalSizeGenerator : public SizeGenerator { boost::variate_generator > gen_; }; -uint8_t* gen_uniform_buffer(uint32_t buf_len, uint8_t c) { +boost::shared_array 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(buf); } -uint8_t* gen_compressible_buffer(uint32_t buf_len) { +boost::shared_array 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(buf); } -uint8_t* gen_random_buffer(uint32_t buf_len) { +boost::shared_array gen_random_buffer(uint32_t buf_len) { uint8_t* buf = new uint8_t[buf_len]; boost::uniform_smallint 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(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 buf, uint32_t buf_len) { boost::shared_ptr membuf(new TMemoryBuffer()); boost::shared_ptr zlib_trans(new TZlibTransport(membuf)); - zlib_trans->write(buf, buf_len); + zlib_trans->write(buf.get(), buf_len); zlib_trans->finish(); boost::shared_array 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 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 membuf(new TMemoryBuffer()); boost::shared_ptr 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 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 buf, uint32_t buf_len) { // Make sure we still get that "not complete" error if // it really isn't complete. boost::shared_ptr membuf(new TMemoryBuffer()); boost::shared_ptr 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 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 buf, uint32_t buf_len, const boost::shared_ptr& write_gen, const boost::shared_ptr& 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 buf, uint32_t buf_len) { // Verify checksum checking. boost::shared_ptr membuf(new TMemoryBuffer()); boost::shared_ptr 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 buf, uint32_t buf_len) { // write some data boost::shared_ptr membuf(new TMemoryBuffer()); boost::shared_ptr 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& buf, uint32_t buf_len, const char* name) { ADD_TEST_CASE(suite, name, test_write_then_read, buf, buf_len);