Repository: orc Updated Branches: refs/heads/master 0a815b5ce -> e159f05c0
ORC-372: Enable valgrind for C++ travis-ci tests Fixes #277 Signed-off-by: Owen O'Malley <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/orc/repo Commit: http://git-wip-us.apache.org/repos/asf/orc/commit/e159f05c Tree: http://git-wip-us.apache.org/repos/asf/orc/tree/e159f05c Diff: http://git-wip-us.apache.org/repos/asf/orc/diff/e159f05c Branch: refs/heads/master Commit: e159f05c0f95171ca9a3d01aefff4d8e22b574af Parents: 0a815b5 Author: Deepak Majeti <[email protected]> Authored: Sun Jun 3 12:52:39 2018 -0400 Committer: Owen O'Malley <[email protected]> Committed: Tue Jun 5 13:40:44 2018 -0700 ---------------------------------------------------------------------- .travis.yml | 11 ++- CMakeLists.txt | 4 ++ c++/src/Compression.cc | 9 +++ c++/src/TypeImpl.cc | 131 +++++++++++++++++++----------------- c++/src/TypeImpl.hh | 40 +++++------ c++/test/CMakeLists.txt | 7 +- c++/test/MemoryOutputStream.cc | 2 +- c++/test/TestDriver.cc | 1 + tools/test/CMakeLists.txt | 6 +- tools/test/ToolTest.cc | 1 + 10 files changed, 128 insertions(+), 84 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/.travis.yml ---------------------------------------------------------------------- diff --git a/.travis.yml b/.travis.yml index 73537de..a466deb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,12 +1,21 @@ language: cpp sudo: false dist: trusty +addons: + apt: + packages: + - valgrind matrix: fast_finish: true include: - compiler: gcc os: linux + script: + - mkdir build + - cd build + - cmake -DANALYZE_JAVA=ON -DTEST_VALGRIND_MEMCHECK=ON .. + - travis_wait 35 make package test-out - compiler: clang os: linux addons: @@ -44,4 +53,4 @@ script: - mkdir build - cd build - cmake -DANALYZE_JAVA=ON .. - - make package test-out \ No newline at end of file + - make package test-out http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/CMakeLists.txt b/CMakeLists.txt index ce60c1a..7757969 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,6 +41,10 @@ option(BUILD_TOOLS "Build the tools" ON) +option(TEST_VALGRIND_MEMCHECK + "Run the test suite using valgrind --tool=memcheck" + OFF) + option(INSTALL_VENDORED_LIBS "Install vendored thirdparty libraries along with liborc" ON) http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/c++/src/Compression.cc ---------------------------------------------------------------------- diff --git a/c++/src/Compression.cc b/c++/src/Compression.cc index 518bddc..6bd0220 100644 --- a/c++/src/Compression.cc +++ b/c++/src/Compression.cc @@ -199,6 +199,10 @@ namespace orc { uint64_t blockSize, MemoryPool& pool); + virtual ~ZlibCompressionStream() override { + end(); + } + virtual std::string getName() const override; protected: @@ -206,6 +210,7 @@ namespace orc { private: void init(); + void end(); z_stream strm; }; @@ -282,6 +287,10 @@ DIAGNOSTIC_PUSH } } + void ZlibCompressionStream::end() { + (void)deflateEnd(&strm); + } + DIAGNOSTIC_PUSH enum DecompressState { DECOMPRESS_HEADER, http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/c++/src/TypeImpl.cc ---------------------------------------------------------------------- diff --git a/c++/src/TypeImpl.cc b/c++/src/TypeImpl.cc index 63a5038..8f30a8a 100644 --- a/c++/src/TypeImpl.cc +++ b/c++/src/TypeImpl.cc @@ -258,31 +258,34 @@ namespace orc { case STRUCT: { StructVectorBatch *result = new StructVectorBatch(capacity, memoryPool); + std::unique_ptr<ColumnVectorBatch> return_value = std::unique_ptr<ColumnVectorBatch>(result); for(uint64_t i=0; i < getSubtypeCount(); ++i) { result->fields.push_back(getSubtype(i)-> createRowBatch(capacity, memoryPool).release()); } - return std::unique_ptr<ColumnVectorBatch>(result); + return return_value; } case LIST: { ListVectorBatch* result = new ListVectorBatch(capacity, memoryPool); + std::unique_ptr<ColumnVectorBatch> return_value = std::unique_ptr<ColumnVectorBatch>(result); if (getSubtype(0) != nullptr) { result->elements = getSubtype(0)->createRowBatch(capacity, memoryPool); } - return std::unique_ptr<ColumnVectorBatch>(result); + return return_value; } case MAP: { MapVectorBatch* result = new MapVectorBatch(capacity, memoryPool); + std::unique_ptr<ColumnVectorBatch> return_value = std::unique_ptr<ColumnVectorBatch>(result); if (getSubtype(0) != nullptr) { result->keys = getSubtype(0)->createRowBatch(capacity, memoryPool); } if (getSubtype(1) != nullptr) { result->elements = getSubtype(1)->createRowBatch(capacity, memoryPool); } - return std::unique_ptr<ColumnVectorBatch>(result); + return return_value; } case DECIMAL: { @@ -297,12 +300,13 @@ namespace orc { case UNION: { UnionVectorBatch *result = new UnionVectorBatch(capacity, memoryPool); + std::unique_ptr<ColumnVectorBatch> return_value = std::unique_ptr<ColumnVectorBatch>(result); for(uint64_t i=0; i < getSubtypeCount(); ++i) { result->children.push_back(getSubtype(i)->createRowBatch(capacity, memoryPool) .release()); } - return std::unique_ptr<ColumnVectorBatch>(result); + return return_value; } default: @@ -330,16 +334,18 @@ namespace orc { std::unique_ptr<Type> createListType(std::unique_ptr<Type> elements) { TypeImpl* result = new TypeImpl(LIST); + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(result); result->addChildType(std::move(elements)); - return std::unique_ptr<Type>(result); + return return_value; } std::unique_ptr<Type> createMapType(std::unique_ptr<Type> key, std::unique_ptr<Type> value) { TypeImpl* result = new TypeImpl(MAP); + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(result); result->addChildType(std::move(key)); result->addChildType(std::move(value)); - return std::unique_ptr<Type>(result); + return return_value; } std::unique_ptr<Type> createUnionType() { @@ -379,6 +385,7 @@ namespace orc { case proto::Type_Kind_MAP: case proto::Type_Kind_UNION: { TypeImpl* result = new TypeImpl(static_cast<TypeKind>(type.kind())); + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(result); if (type.kind() == proto::Type_Kind_LIST && type.subtypes_size() != 1) throw ParseError("Illegal LIST type that doesn't contain one subtype"); if (type.kind() == proto::Type_Kind_MAP && type.subtypes_size() != 2) @@ -390,11 +397,12 @@ namespace orc { (type.subtypes(i))), footer)); } - return std::unique_ptr<Type>(result); + return return_value; } case proto::Type_Kind_STRUCT: { TypeImpl* result = new TypeImpl(STRUCT); + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(result); uint64_t size = static_cast<uint64_t>(type.subtypes_size()); std::vector<Type*> typeList(size); std::vector<std::string> fieldList(size); @@ -404,7 +412,7 @@ namespace orc { (type.subtypes(i))), footer)); } - return std::unique_ptr<Type>(result); + return return_value; } default: throw NotImplementedYet("Unknown type kind"); @@ -498,76 +506,80 @@ namespace orc { } ORC_UNIQUE_PTR<Type> Type::buildTypeFromString(const std::string& input) { - std::vector<std::pair<std::string, Type*> > res = + std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type> > > res = TypeImpl::parseType(input, 0, input.size()); if (res.size() != 1) { throw std::logic_error("Invalid type string."); } - return ORC_UNIQUE_PTR<Type>(res[0].second); + return std::move(res[0].second); } - Type* TypeImpl::parseArrayType(const std::string &input, - size_t start, - size_t end) { + std::unique_ptr<Type> TypeImpl::parseArrayType(const std::string &input, + size_t start, + size_t end) { TypeImpl* arrayType = new TypeImpl(LIST); - std::vector<std::pair<std::string, Type*> > v = + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(arrayType); + std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type> > > v = TypeImpl::parseType(input, start, end); if (v.size() != 1) { throw std::logic_error("Array type must contain exactly one sub type."); } - arrayType->addChildType(ORC_UNIQUE_PTR<Type>(v[0].second)); - return arrayType; + arrayType->addChildType(std::move(v[0].second)); + return return_value; } - Type* TypeImpl::parseMapType(const std::string &input, - size_t start, - size_t end) { + std::unique_ptr<Type> TypeImpl::parseMapType(const std::string &input, + size_t start, + size_t end) { TypeImpl * mapType = new TypeImpl(MAP); - std::vector<std::pair<std::string, Type*> > v = + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(mapType); + std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type> > > v = TypeImpl::parseType(input, start, end); if (v.size() != 2) { throw std::logic_error( "Map type must contain exactly two sub types."); } - mapType->addChildType(ORC_UNIQUE_PTR<Type>(v[0].second)); - mapType->addChildType(ORC_UNIQUE_PTR<Type>(v[1].second)); - return mapType; + mapType->addChildType(std::move(v[0].second)); + mapType->addChildType(std::move(v[1].second)); + return return_value; } - Type* TypeImpl::parseStructType(const std::string &input, - size_t start, - size_t end) { + std::unique_ptr<Type> TypeImpl::parseStructType(const std::string &input, + size_t start, + size_t end) { TypeImpl* structType = new TypeImpl(STRUCT); - std::vector<std::pair<std::string, Type*> > v = + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(structType); + std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type>> > v = TypeImpl::parseType(input, start, end); if (v.size() == 0) { throw std::logic_error( "Struct type must contain at least one sub type."); } for (size_t i = 0; i < v.size(); ++i) { - structType->addStructField(v[i].first, ORC_UNIQUE_PTR<Type>(v[i].second)); + structType->addStructField(v[i].first, std::move(v[i].second)); } - return structType; + return return_value; } - Type* TypeImpl::parseUnionType(const std::string &input, - size_t start, - size_t end) { + std::unique_ptr<Type> TypeImpl::parseUnionType(const std::string &input, + size_t start, + size_t end) { TypeImpl* unionType = new TypeImpl(UNION); - std::vector<std::pair<std::string, Type*> > v = + std::unique_ptr<Type> return_value = std::unique_ptr<Type>(unionType); + std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type> > > v = TypeImpl::parseType(input, start, end); if (v.size() == 0) { throw std::logic_error("Union type must contain at least one sub type."); } for (size_t i = 0; i < v.size(); ++i) { - unionType->addChildType(ORC_UNIQUE_PTR<Type>(v[i].second)); + unionType->addChildType(std::move(v[i].second)); } - return unionType; + return return_value; } - Type* TypeImpl::parseDecimalType(const std::string &input, - size_t start, - size_t end) { + std::unique_ptr<Type> TypeImpl::parseDecimalType(const std::string &input, + size_t start, + size_t end) { size_t sep = input.find(',', start); if (sep + 1 >= end || sep == std::string::npos) { throw std::logic_error("Decimal type must specify precision and scale."); @@ -576,33 +588,33 @@ namespace orc { static_cast<uint64_t>(atoi(input.substr(start, sep - start).c_str())); uint64_t scale = static_cast<uint64_t>(atoi(input.substr(sep + 1, end - sep - 1).c_str())); - return new TypeImpl(DECIMAL, precision, scale); + return std::unique_ptr<Type>(new TypeImpl(DECIMAL, precision, scale)); } - Type* TypeImpl::parseCategory(std::string category, - const std::string &input, - size_t start, - size_t end) { + std::unique_ptr<Type> TypeImpl::parseCategory(std::string category, + const std::string &input, + size_t start, + size_t end) { if (category == "boolean") { - return new TypeImpl(BOOLEAN); + return std::unique_ptr<Type>(new TypeImpl(BOOLEAN)); } else if (category == "tinyint") { - return new TypeImpl(BYTE); + return std::unique_ptr<Type>(new TypeImpl(BYTE)); } else if (category == "smallint") { - return new TypeImpl(SHORT); + return std::unique_ptr<Type>(new TypeImpl(SHORT)); } else if (category == "int") { - return new TypeImpl(INT); + return std::unique_ptr<Type>(new TypeImpl(INT)); } else if (category == "bigint") { - return new TypeImpl(LONG); + return std::unique_ptr<Type>(new TypeImpl(LONG)); } else if (category == "float") { - return new TypeImpl(FLOAT); + return std::unique_ptr<Type>(new TypeImpl(FLOAT)); } else if (category == "double") { - return new TypeImpl(DOUBLE); + return std::unique_ptr<Type>(new TypeImpl(DOUBLE)); } else if (category == "string") { - return new TypeImpl(STRING); + return std::unique_ptr<Type>(new TypeImpl(STRING)); } else if (category == "binary") { - return new TypeImpl(BINARY); + return std::unique_ptr<Type>(new TypeImpl(BINARY)); } else if (category == "timestamp") { - return new TypeImpl(TIMESTAMP); + return std::unique_ptr<Type>(new TypeImpl(TIMESTAMP)); } else if (category == "array") { return parseArrayType(input, start, end); } else if (category == "map") { @@ -614,26 +626,26 @@ namespace orc { } else if (category == "decimal") { return parseDecimalType(input, start, end); } else if (category == "date") { - return new TypeImpl(DATE); + return std::unique_ptr<Type>(new TypeImpl(DATE)); } else if (category == "varchar") { uint64_t maxLength = static_cast<uint64_t>( atoi(input.substr(start, end - start).c_str())); - return new TypeImpl(VARCHAR, maxLength); + return std::unique_ptr<Type>(new TypeImpl(VARCHAR, maxLength)); } else if (category == "char") { uint64_t maxLength = static_cast<uint64_t>( atoi(input.substr(start, end - start).c_str())); - return new TypeImpl(CHAR, maxLength); + return std::unique_ptr<Type>(new TypeImpl(CHAR, maxLength)); } else { throw std::logic_error("Unknown type " + category); } } - std::vector<std::pair<std::string, Type *> > TypeImpl::parseType( + std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type> > > TypeImpl::parseType( const std::string &input, size_t start, size_t end) { std::string types = input.substr(start, end - start); - std::vector<std::pair<std::string, Type *> > res; + std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type> > > res; size_t pos = 0; while (pos < types.size()) { @@ -680,8 +692,7 @@ namespace orc { } std::string category = types.substr(pos, endPos - pos); - Type* type = parseCategory(category, types, endPos + 1, nextPos); - res.push_back(std::make_pair(fieldName, type)); + res.push_back(std::make_pair(fieldName, parseCategory(category, types, endPos + 1, nextPos))); if (nextPos < types.size() && (types[nextPos] == ')' || types[nextPos] == '>')) { pos = nextPos + 2; http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/c++/src/TypeImpl.hh ---------------------------------------------------------------------- diff --git a/c++/src/TypeImpl.hh b/c++/src/TypeImpl.hh index ee34f78..4cf50f3 100644 --- a/c++/src/TypeImpl.hh +++ b/c++/src/TypeImpl.hh @@ -98,7 +98,7 @@ namespace orc { */ void addChildType(std::unique_ptr<Type> childType); - static std::vector<std::pair<std::string, Type *> > parseType( + static std::vector<std::pair<std::string, std::unique_ptr<Type> > > parseType( const std::string &input, size_t start, size_t end); @@ -122,9 +122,9 @@ namespace orc { * @param start start position of the input string * @param end end position of the input string */ - static Type* parseArrayType(const std::string &input, - size_t start, - size_t end); + static std::unique_ptr<Type> parseArrayType(const std::string &input, + size_t start, + size_t end); /** * Parse map type from string @@ -132,9 +132,9 @@ namespace orc { * @param start start position of the input string * @param end end position of the input string */ - static Type* parseMapType(const std::string &input, - size_t start, - size_t end); + static std::unique_ptr<Type> parseMapType(const std::string &input, + size_t start, + size_t end); /** * Parse struct type from string @@ -142,9 +142,9 @@ namespace orc { * @param start start position of the input string * @param end end position of the input string */ - static Type* parseStructType(const std::string &input, - size_t start, - size_t end); + static std::unique_ptr<Type> parseStructType(const std::string &input, + size_t start, + size_t end); /** * Parse union type from string @@ -152,9 +152,9 @@ namespace orc { * @param start start position of the input string * @param end end position of the input string */ - static Type* parseUnionType(const std::string &input, - size_t start, - size_t end); + static std::unique_ptr<Type> parseUnionType(const std::string &input, + size_t start, + size_t end); /** * Parse decimal type from string @@ -162,9 +162,9 @@ namespace orc { * @param start start position of the input string * @param end end position of the input string */ - static Type* parseDecimalType(const std::string &input, - size_t start, - size_t end); + static std::unique_ptr<Type> parseDecimalType(const std::string &input, + size_t start, + size_t end); /** * Parse type for a category @@ -173,10 +173,10 @@ namespace orc { * @param start start position of the input string * @param end end position of the input string */ - static Type* parseCategory(std::string category, - const std::string &input, - size_t start, - size_t end); + static std::unique_ptr<Type> parseCategory(std::string category, + const std::string &input, + size_t start, + size_t end); }; std::unique_ptr<Type> convertType(const proto::Type& type, http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/c++/test/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/c++/test/CMakeLists.txt b/c++/test/CMakeLists.txt index 8e7c878..0a04aaf 100644 --- a/c++/test/CMakeLists.txt +++ b/c++/test/CMakeLists.txt @@ -64,7 +64,12 @@ target_link_libraries (create-test-files protobuf ) -add_test (NAME orc-test COMMAND orc-test) +if (TEST_VALGRIND_MEMCHECK) + add_test (orc-test + valgrind --tool=memcheck --leak-check=full --error-exitcode=1 ./orc-test) +else () + add_test (NAME orc-test COMMAND orc-test) +endif () if (WIN32) set_property( http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/c++/test/MemoryOutputStream.cc ---------------------------------------------------------------------- diff --git a/c++/test/MemoryOutputStream.cc b/c++/test/MemoryOutputStream.cc index 972b5af..74940af 100644 --- a/c++/test/MemoryOutputStream.cc +++ b/c++/test/MemoryOutputStream.cc @@ -21,7 +21,7 @@ namespace orc { MemoryOutputStream::~MemoryOutputStream() { - // PASS + delete[] data; } void MemoryOutputStream::write(const void* buf, size_t size) { http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/c++/test/TestDriver.cc ---------------------------------------------------------------------- diff --git a/c++/test/TestDriver.cc b/c++/test/TestDriver.cc index a34ff07..63f0f2b 100644 --- a/c++/test/TestDriver.cc +++ b/c++/test/TestDriver.cc @@ -29,5 +29,6 @@ GTEST_API_ int main(int argc, char **argv) { std::cout << "ORC version: " << ORC_VERSION << "\n"; testing::InitGoogleTest(&argc, argv); int result = RUN_ALL_TESTS(); + google::protobuf::ShutdownProtobufLibrary(); return result; } http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/tools/test/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/tools/test/CMakeLists.txt b/tools/test/CMakeLists.txt index 59f60f1..505fc8d 100644 --- a/tools/test/CMakeLists.txt +++ b/tools/test/CMakeLists.txt @@ -42,4 +42,8 @@ target_link_libraries (tool-test ${GTEST_LIBRARIES} ) -add_test (tool-test tool-test ${EXAMPLE_DIRECTORY} ${PROJECT_BINARY_DIR}) +if (TEST_VALGRIND_MEMCHECK) + add_test (tool-test valgrind --tool=memcheck --leak-check=full --error-exitcode=1 ./tool-test ${EXAMPLE_DIRECTORY} ${PROJECT_BINARY_DIR}) +else () + add_test (tool-test tool-test ${EXAMPLE_DIRECTORY} ${PROJECT_BINARY_DIR}) +endif () http://git-wip-us.apache.org/repos/asf/orc/blob/e159f05c/tools/test/ToolTest.cc ---------------------------------------------------------------------- diff --git a/tools/test/ToolTest.cc b/tools/test/ToolTest.cc index 10b7a5f..8008a56 100644 --- a/tools/test/ToolTest.cc +++ b/tools/test/ToolTest.cc @@ -58,6 +58,7 @@ GTEST_API_ int main(int argc, char **argv) { } testing::InitGoogleTest(&argc, argv); int result = RUN_ALL_TESTS(); + google::protobuf::ShutdownProtobufLibrary(); return result; }
