From 70b2db19a8d5915e0466dc3e7b3af3af35a81c93 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 8 Jul 2022 09:47:33 +0200 Subject: [PATCH 1/4] Remove assertion test - Code cleanup - closes https://github.com/assimp/assimp/issues/4626 --- code/AssetLib/Step/STEPFile.h | 140 +++++++++++----------------------- 1 file changed, 46 insertions(+), 94 deletions(-) diff --git a/code/AssetLib/Step/STEPFile.h b/code/AssetLib/Step/STEPFile.h index e09faad98..cf5d732d5 100644 --- a/code/AssetLib/Step/STEPFile.h +++ b/code/AssetLib/Step/STEPFile.h @@ -2,8 +2,7 @@ Open Asset Import Library (assimp) ---------------------------------------------------------------------- -Copyright (c) 2006-2020, assimp team - +Copyright (c) 2006-2022, assimp team All rights reserved. @@ -59,7 +58,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # pragma warning(disable : 4127 4456 4245 4512 ) #endif // _MSC_VER -// #if _MSC_VER > 1500 || (defined __GNUC___) # define ASSIMP_STEP_USE_UNORDERED_MULTIMAP #else @@ -99,13 +97,9 @@ namespace EXPRESS { class DataType; class UNSET; /*: public DataType */ class ISDERIVED; /*: public DataType */ -// class REAL; /*: public DataType */ class ENUM; /*: public DataType */ -// class STRING; /*: public DataType */ -// class INTEGER; /*: public DataType */ class ENTITY; /*: public DataType */ class LIST; /*: public DataType */ -// class SELECT; /*: public DataType */ // a conversion schema is not exactly an EXPRESS schema, rather it // is a list of pointers to conversion functions to build up the @@ -127,7 +121,8 @@ namespace STEP { // ------------------------------------------------------------------------------- /** Exception class used by the STEP loading & parsing code. It is typically - * coupled with a line number. */ + * coupled with a line number. + */ // ------------------------------------------------------------------------------- struct SyntaxError : DeadlyImportError { enum : uint64_t { @@ -139,8 +134,9 @@ struct SyntaxError : DeadlyImportError { // ------------------------------------------------------------------------------- /** Exception class used by the STEP loading & parsing code when a type - * error (i.e. an entity expects a string but receives a bool) occurs. - * It is typically coupled with both an entity id and a line number.*/ + * error (i.e. an entity expects a string but receives a bool) occurs. + * It is typically coupled with both an entity id and a line number. + */ // ------------------------------------------------------------------------------- struct TypeError : DeadlyImportError { enum : uint64_t { @@ -167,10 +163,8 @@ public: typedef std::shared_ptr Out; public: - virtual ~DataType() { - } + virtual ~DataType() = default -public: template const T &To() const { return dynamic_cast(*this); @@ -206,16 +200,14 @@ public: public: /** parse a variable from a string and set 'inout' to the character - * behind the last consumed character. An optional schema enables, - * if specified, automatic conversion of custom data types. - * - * @throw SyntaxError - */ + * behind the last consumed character. An optional schema enables, + * if specified, automatic conversion of custom data types. + * + * @throw SyntaxError + */ static std::shared_ptr Parse(const char *&inout, uint64_t line = SyntaxError::LINE_NOT_SPECIFIED, const EXPRESS::ConversionSchema *schema = NULL); - -public: }; typedef DataType SELECT; @@ -238,7 +230,8 @@ private: }; // ------------------------------------------------------------------------------- -/** Shared implementation for some of the primitive data type, i.e. int, float */ +/** Shared implementation for some of the primitive data type, i.e. int, float + */ // ------------------------------------------------------------------------------- template class PrimitiveDataType : public DataType { @@ -247,7 +240,7 @@ public: // expose this data type to the user. typedef T Out; - PrimitiveDataType() {} + PrimitiveDataType() = default; PrimitiveDataType(const T &val) : val(val) {} @@ -280,28 +273,18 @@ class ENUMERATION : public STRING { public: ENUMERATION(const std::string &val) : STRING(val) {} - -private: }; typedef ENUMERATION BOOLEAN; // ------------------------------------------------------------------------------- -/** This is just a reference to an entity/object somewhere else */ +/** This is just a reference to an entity/object somewhere else + */ // ------------------------------------------------------------------------------- class ENTITY : public PrimitiveDataType { public: - ENTITY(uint64_t val) : - PrimitiveDataType(val) { - ai_assert(val != 0); - } - - ENTITY() : - PrimitiveDataType(TypeError::ENTITY_NOT_SPECIFIED) { - // empty - } - -private: + ENTITY(uint64_t val) : PrimitiveDataType(val) {} + ENTITY() : PrimitiveDataType(TypeError::ENTITY_NOT_SPECIFIED) {} }; // ------------------------------------------------------------------------------- @@ -319,7 +302,8 @@ public: } public: - /** @see DaraType::Parse */ + /** @see DaraType::Parse + */ static std::shared_ptr Parse(const char *&inout, uint64_t line = SyntaxError::LINE_NOT_SPECIFIED, const EXPRESS::ConversionSchema *schema = NULL); @@ -331,29 +315,20 @@ private: class BINARY : public PrimitiveDataType { public: - BINARY(uint32_t val) : - PrimitiveDataType(val) { - // empty - } - - BINARY() : - PrimitiveDataType(TypeError::ENTITY_NOT_SPECIFIED_32) { - // empty - } + BINARY(uint32_t val) : PrimitiveDataType(val) {} + BINARY() : PrimitiveDataType(TypeError::ENTITY_NOT_SPECIFIED_32) {} }; // ------------------------------------------------------------------------------- /* Not exactly a full EXPRESS schema but rather a list of conversion functions - * to extract valid C++ objects out of a STEP file. Those conversion functions - * may, however, perform further schema validations. */ + * to extract valid C++ objects out of a STEP file. Those conversion functions + * may, however, perform further schema validations. + */ // ------------------------------------------------------------------------------- class ConversionSchema { public: struct SchemaEntry { - SchemaEntry(const char *name, ConvertObjectProc func) : - mName(name), mFunc(func) { - // empty - } + SchemaEntry(const char *name, ConvertObjectProc func) : mName(name), mFunc(func) {} const char *mName; ConvertObjectProc mFunc; @@ -366,8 +341,7 @@ public: *this = schemas; } - ConversionSchema() { - } + ConversionSchema() = default ConvertObjectProc GetConverterProc(const std::string &name) const { ConverterMap::const_iterator it = converters.find(name); @@ -399,8 +373,9 @@ private: // ------------------------------------------------------------------------------ /** Bundle all the relevant info from a STEP header, parts of which may later - * be plainly dumped to the logfile, whereas others may help the caller pick an - * appropriate loading strategy.*/ + * be plainly dumped to the logfile, whereas others may help the caller pick an + * appropriate loading strategy. + */ // ------------------------------------------------------------------------------ struct HeaderInfo { std::string timestamp; @@ -409,18 +384,14 @@ struct HeaderInfo { }; // ------------------------------------------------------------------------------ -/** Base class for all concrete object instances */ +/** Base class for all concrete object instances + */ // ------------------------------------------------------------------------------ class Object { public: - Object(const char *classname = "unknown") : - id(0), classname(classname) { - // empty - } + Object(const char *classname = "unknown") : id(0), classname(classname) {} - virtual ~Object() { - // empty - } + virtual ~Object() = default; // utilities to simplify casting to concrete types template @@ -469,26 +440,15 @@ size_t GenericFill(const STEP::DB &db, const EXPRESS::LIST ¶ms, T *in); // ------------------------------------------------------------------------------ template struct ObjectHelper : virtual Object { - ObjectHelper() : - aux_is_derived(0) { - // empty - } + ObjectHelper() : aux_is_derived(0) {} static Object *Construct(const STEP::DB &db, const EXPRESS::LIST ¶ms) { // make sure we don't leak if Fill() throws an exception std::unique_ptr impl(new TDerived()); // GenericFill is undefined so we need to have a specialization - const size_t num_args = GenericFill(db, params, &*impl); - (void)num_args; + static_cast(GenericFill(db, params, &*impl)); - // the following check is commented because it will always trigger if - // parts of the entities are generated with dummy wrapper code. - // This is currently done to reduce the size of the loader - // code. - //if (num_args != params.GetSize() && impl->GetClassName() != "NotImplemented") { - // DefaultLogger::get()->debug("STEP: not all parameters consumed"); - //} return impl.release(); } @@ -502,15 +462,9 @@ struct ObjectHelper : virtual Object { // ------------------------------------------------------------------------------ template struct Maybe { - Maybe() : - have() { - // empty - } + Maybe() : have() {} - explicit Maybe(const T &ptr) : - ptr(ptr), have(true) { - // empty - } + explicit Maybe(const T &ptr) : ptr(ptr), have(true) {} void flag_invalid() { have = false; @@ -557,7 +511,8 @@ private: // ------------------------------------------------------------------------------ /** A LazyObject is created when needed. Before this happens, we just keep - the text line that contains the object definition. */ + * the text line that contains the object definition. + */ // ------------------------------------------------------------------------------- class LazyObject { friend class DB; @@ -649,10 +604,7 @@ inline bool operator==(const std::pair> &l template struct Lazy { typedef Lazy Out; - Lazy(const LazyObject *obj = nullptr) : - obj(obj) { - // empty - } + Lazy(const LazyObject *obj = nullptr) : obj(obj) {} operator const T *() const { return obj->ToPtr(); @@ -785,8 +737,9 @@ inline void GenericConvert(ListOf &a, const std::shared_ptr stream); @@ -873,7 +826,7 @@ public: if (it != objects_bytype.end() && (*it).second.size()) { return *(*it).second.begin(); } - return NULL; + return nullptr; } // same, but raise an exception if the object doesn't exist and return a reference @@ -965,7 +918,6 @@ private: #endif // _MSC_VER } // namespace STEP - } // namespace Assimp #endif // INCLUDED_AI_STEPFILE_H From e59b8fb4483967bbc5b87f447137ade74f26cf5b Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 8 Jul 2022 09:50:04 +0200 Subject: [PATCH 2/4] Fix typo --- code/AssetLib/Step/STEPFile.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/Step/STEPFile.h b/code/AssetLib/Step/STEPFile.h index cf5d732d5..ce7b357ad 100644 --- a/code/AssetLib/Step/STEPFile.h +++ b/code/AssetLib/Step/STEPFile.h @@ -163,7 +163,7 @@ public: typedef std::shared_ptr Out; public: - virtual ~DataType() = default + virtual ~DataType() = default; template const T &To() const { @@ -341,7 +341,7 @@ public: *this = schemas; } - ConversionSchema() = default + ConversionSchema() = default; ConvertObjectProc GetConverterProc(const std::string &name) const { ConverterMap::const_iterator it = converters.find(name); From e254f80a3a6884f64895767b7a803853a5c585c6 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 8 Jul 2022 10:17:11 +0200 Subject: [PATCH 3/4] Fix memory leak in D3MFOpcPackage - closes https://github.com/assimp/assimp/issues/4628 --- code/AssetLib/3MF/D3MFOpcPackage.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/AssetLib/3MF/D3MFOpcPackage.cpp b/code/AssetLib/3MF/D3MFOpcPackage.cpp index f88039ae8..fea49a6bd 100644 --- a/code/AssetLib/3MF/D3MFOpcPackage.cpp +++ b/code/AssetLib/3MF/D3MFOpcPackage.cpp @@ -186,6 +186,9 @@ D3MFOpcPackage::D3MFOpcPackage(IOSystem *pIOHandler, const std::string &rFile) : D3MFOpcPackage::~D3MFOpcPackage() { mZipArchive->Close(mRootStream); delete mZipArchive; + for (auto tex : mEmbeddedTextures) { + delete mEmbeddedTextures[i]; + } } IOStream *D3MFOpcPackage::RootStream() const { From c5dfcac08a26c554494a22dc80620e716d929fdc Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 8 Jul 2022 10:25:40 +0200 Subject: [PATCH 4/4] Update D3MFOpcPackage.cpp --- code/AssetLib/3MF/D3MFOpcPackage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/3MF/D3MFOpcPackage.cpp b/code/AssetLib/3MF/D3MFOpcPackage.cpp index fea49a6bd..a2182dc29 100644 --- a/code/AssetLib/3MF/D3MFOpcPackage.cpp +++ b/code/AssetLib/3MF/D3MFOpcPackage.cpp @@ -187,7 +187,7 @@ D3MFOpcPackage::~D3MFOpcPackage() { mZipArchive->Close(mRootStream); delete mZipArchive; for (auto tex : mEmbeddedTextures) { - delete mEmbeddedTextures[i]; + delete tex; } }