From f3ec105f1408974b6d7cd83fceb4022b9c51dcce Mon Sep 17 00:00:00 2001 From: Pierre-Anthony Lemieux Date: Tue, 18 Sep 2018 23:31:45 +0200 Subject: [PATCH] Fixed memory leaks (#129, #130) Added memory leak checking to Travis script --- .travis.yml | 2 + regxmllib/CMakeLists.txt | 2 + regxmllib/CTestConfig.cmake | 1 + .../cpp/com/sandflow/smpte/klv/KLVStream.cpp | 2 +- .../cpp/com/sandflow/smpte/klv/LocalSet.cpp | 4 +- .../com/sandflow/smpte/mxf/PartitionPack.cpp | 4 +- .../main/cpp/com/sandflow/smpte/mxf/Set.cpp | 2 +- .../sandflow/smpte/regxml/FragmentBuilder.cpp | 2 +- .../smpte/regxml/dict/MetaDictionary.cpp | 42 +++++++++++-------- .../regxml/dict/importers/XMLImporter.cpp | 4 +- .../main/cpp/com/sandflow/smpte/util/AUID.cpp | 4 +- .../smpte/dict/MetaDictionaryTest.cpp | 3 +- 12 files changed, 42 insertions(+), 30 deletions(-) create mode 100644 regxmllib/CTestConfig.cmake diff --git a/.travis.yml b/.travis.yml index ed8d61e..628e643 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ matrix: apt: packages: - libxerces-c-dev + - valgrind script: - mkdir regxmllib/build @@ -12,6 +13,7 @@ matrix: - cmake .. - cmake --build . - ctest . + - ctest -T memcheck . - sudo make install - language: java diff --git a/regxmllib/CMakeLists.txt b/regxmllib/CMakeLists.txt index f0ed1c9..a000fc1 100644 --- a/regxmllib/CMakeLists.txt +++ b/regxmllib/CMakeLists.txt @@ -28,6 +28,8 @@ endforeach() enable_testing() +include(CTest) + file( GLOB_RECURSE UNIT_TESTS src/test/cpp/*.cpp ) file(COPY "src/test/resources" DESTINATION "${CMAKE_BINARY_DIR}") diff --git a/regxmllib/CTestConfig.cmake b/regxmllib/CTestConfig.cmake new file mode 100644 index 0000000..48f8be8 --- /dev/null +++ b/regxmllib/CTestConfig.cmake @@ -0,0 +1 @@ +set(MEMORYCHECK_COMMAND_OPTIONS "--show-leak-kinds=definite --leak-check=full --error-exitcode=1") diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/klv/KLVStream.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/klv/KLVStream.cpp index 2e8a6bd..a49676a 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/klv/KLVStream.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/klv/KLVStream.cpp @@ -102,7 +102,7 @@ namespace rxml { throw std::ios_base::failure("Max BER length exceeded"); } - unsigned char *buf = new unsigned char[bersz]; + unsigned char buf[8]; this->read((char*)buf, bersz); diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/klv/LocalSet.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/klv/LocalSet.cpp index 7ed1a0c..510640d 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/klv/LocalSet.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/klv/LocalSet.cpp @@ -61,14 +61,14 @@ namespace rxml { if (!t.getKey().isUL()) { - throw new KLVException("Triplet key " + rxml::to_string(t.getKey()) + " is not a UL"); + throw KLVException("Triplet key " + rxml::to_string(t.getKey()) + " is not a UL"); } UL ul = t.getKey().asUL(); if (!ul.isLocalSet()) { - throw new KLVException("Triplet with key " + rxml::to_string(t.getKey()) + " is not a Local Set"); + throw KLVException("Triplet with key " + rxml::to_string(t.getKey()) + " is not a Local Set"); } membuf mb((char*)t.getValue(), (char*)t.getValue() + t.getLength()); diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/PartitionPack.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/PartitionPack.cpp index 77488c5..0ca610d 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/PartitionPack.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/PartitionPack.cpp @@ -34,7 +34,7 @@ namespace rxml { if (!t.getKey().isUL()) { - throw new MXFException("Triplet key " + rxml::to_string(t.getKey()) + " is not a UL"); + throw MXFException("Triplet key " + rxml::to_string(t.getKey()) + " is not a UL"); } UL ul = t.getKey().asUL(); @@ -112,7 +112,7 @@ namespace rxml { this->essenceContainers = kis.readBatch(); } catch (std::exception e) { - throw new MXFException("Error reading partition pack"); + throw MXFException("Error reading partition pack"); } } diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/Set.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/Set.cpp index 37c3ffd..d19d00f 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/Set.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/mxf/Set.cpp @@ -48,7 +48,7 @@ namespace rxml { if (!hasInstanceUID(g)) { - throw new MXFException("Group is missing an instance ID property"); + throw MXFException("Group is missing an instance ID property"); } diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/FragmentBuilder.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/FragmentBuilder.cpp index 8ab7b7a..08c697d 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/FragmentBuilder.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/FragmentBuilder.cpp @@ -392,7 +392,7 @@ namespace rxml { addInformativeComment(element, err.getReason()); } else { - throw new FragmentBuilder::UnknownByteOrderError(ByteOrder_UL.to_string()); + throw FragmentBuilder::UnknownByteOrderError(ByteOrder_UL.to_string()); } } else { diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/MetaDictionary.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/MetaDictionary.cpp index 19b1c69..548bb67 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/MetaDictionary.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/MetaDictionary.cpp @@ -95,6 +95,10 @@ namespace rxml { meta_dictionary.subclassesOf[parentauid].insert(def.identification); } + } else { + + delete def_copy; + } } @@ -111,6 +115,11 @@ namespace rxml { const AUID parentauid = MetaDictionary::createNormalizedAUID(def.memberOf); meta_dictionary.membersOf[parentauid].insert(def.identification); + + } else { + + delete def_copy; + } @@ -118,72 +127,69 @@ namespace rxml { virtual void visit(const PropertyAliasDefinition & def) { - Definition *def_copy = new PropertyAliasDefinition(def); - const AUID parentauid = MetaDictionary::createNormalizedAUID(def.memberOf); meta_dictionary.membersOf[parentauid].insert(def.identification); - } virtual void visit(const EnumerationTypeDefinition &def) { Definition *def_copy = new EnumerationTypeDefinition(def); - _visit(def_copy); + if (! _visit(def_copy)) delete def_copy; } virtual void visit(const CharacterTypeDefinition &def) { Definition *def_copy = new CharacterTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const RenameTypeDefinition & def) { Definition *def_copy = new RenameTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const RecordTypeDefinition & def) { Definition *def_copy = new RecordTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const StringTypeDefinition & def) { Definition *def_copy = new StringTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const LensSerialFloatTypeDefinition & def) { Definition *def_copy = new LensSerialFloatTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const IntegerTypeDefinition & def) { Definition *def_copy = new IntegerTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const StrongReferenceTypeDefinition & def) { Definition *def_copy = new StrongReferenceTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const WeakReferenceTypeDefinition & def) { Definition *def_copy = new WeakReferenceTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const ExtendibleEnumerationTypeDefinition & def) { Definition *def_copy = new ExtendibleEnumerationTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const VariableArrayTypeDefinition & def) { @@ -195,31 +201,31 @@ namespace rxml { virtual void visit(const FixedArrayTypeDefinition & def) { Definition *def_copy = new FixedArrayTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const OpaqueTypeDefinition & def) { Definition *def_copy = new OpaqueTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const IndirectTypeDefinition & def) { Definition *def_copy = new IndirectTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const StreamTypeDefinition & def) { Definition *def_copy = new StreamTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } virtual void visit(const SetTypeDefinition & def) { Definition *def_copy = new SetTypeDefinition(def); - _visit(def_copy); + if (!_visit(def_copy)) delete def_copy; } private: diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/importers/XMLImporter.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/importers/XMLImporter.cpp index 2e96b14..bbf000d 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/importers/XMLImporter.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/regxml/dict/importers/XMLImporter.cpp @@ -263,7 +263,7 @@ namespace rxml { if (!membersElem) { - throw new XMLImporter::Exception("Elements property missing"); + throw XMLImporter::Exception("Elements property missing"); } @@ -344,7 +344,7 @@ namespace rxml { if (!elementsElem) { - throw new XMLImporter::Exception("Elements property missing"); + throw XMLImporter::Exception("Elements property missing"); } diff --git a/regxmllib/src/main/cpp/com/sandflow/smpte/util/AUID.cpp b/regxmllib/src/main/cpp/com/sandflow/smpte/util/AUID.cpp index b8ea7fb..107d961 100644 --- a/regxmllib/src/main/cpp/com/sandflow/smpte/util/AUID.cpp +++ b/regxmllib/src/main/cpp/com/sandflow/smpte/util/AUID.cpp @@ -98,7 +98,7 @@ namespace rxml { UL AUID::asUL() const { if (!isUL()) { - throw new std::invalid_argument("AUID is not a UL"); + throw std::invalid_argument("AUID is not a UL"); } return UL(this->value); @@ -107,7 +107,7 @@ namespace rxml { UUID AUID::asUUID() const { if (!isUUID()) { - throw new std::invalid_argument("AUID is not a UUID"); + throw std::invalid_argument("AUID is not a UUID"); } unsigned char tmp[16]; diff --git a/regxmllib/src/test/cpp/com/sandflow/smpte/dict/MetaDictionaryTest.cpp b/regxmllib/src/test/cpp/com/sandflow/smpte/dict/MetaDictionaryTest.cpp index 62194fa..77a7a12 100644 --- a/regxmllib/src/test/cpp/com/sandflow/smpte/dict/MetaDictionaryTest.cpp +++ b/regxmllib/src/test/cpp/com/sandflow/smpte/dict/MetaDictionaryTest.cpp @@ -302,6 +302,7 @@ int main(int argc, char **argv) { doc->release(); + delete ft; } /* free heap */ @@ -314,7 +315,7 @@ int main(int argc, char **argv) { output->release(); ser->release(); - + delete parser; xercesc::XMLPlatformUtils::Terminate();