You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by ro...@apache.org on 2015/04/07 21:11:54 UTC
thrift git commit: THRIFT-3086 fix a few minor valgrind identified
issues
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 <ji...@simplivity.com>
Authored: Mon Apr 6 21:38:06 2015 -0400
Committer: Roger Meier <ro...@apache.org>
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);