Merge pull request #4605 from assimp/kimkulling/fix_memleak_in_xmlparser

Fix a memory leak

code/AssetLib/3MF/3MFXmlTags.h

@@ -74,6 +74,8 @@ namespace XmlTag {
     const char* const pid = "pid";
     const char* const pindex = "pindex";
     const char* const p1 = "p1";
+    const char *const p2 = "p2";
+    const char *const p3 = "p3";
     const char* const name = "name";
     const char* const type = "type";
     const char* const build = "build";

code/AssetLib/3MF/XmlSerializer.cpp

@@ -64,7 +64,7 @@ bool validateColorString(const char *color) {
     return true;
 }
 
-aiFace ReadTriangle(XmlNode &node) {
+aiFace ReadTriangle(XmlNode &node, unsigned int &texId0, unsigned int &texId1, unsigned int &texId2) {
     aiFace face;
 
     face.mNumIndices = 3;
@@ -73,6 +73,10 @@ aiFace ReadTriangle(XmlNode &node) {
     face.mIndices[1] = static_cast<unsigned int>(std::atoi(node.attribute(XmlTag::v2).as_string()));
     face.mIndices[2] = static_cast<unsigned int>(std::atoi(node.attribute(XmlTag::v3).as_string()));
 
+    texId0 = static_cast<unsigned int>(std::atoi(node.attribute(XmlTag::p1).as_string()));
+    texId1 = static_cast<unsigned int>(std::atoi(node.attribute(XmlTag::p2).as_string()));
+    texId2 = static_cast<unsigned int>(std::atoi(node.attribute(XmlTag::p3).as_string()));
+
     return face;
 }
 
@@ -412,6 +416,8 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) {
             bool hasPid = getNodeAttribute(currentNode, D3MF::XmlTag::pid, pid);
             bool hasP1 = getNodeAttribute(currentNode, D3MF::XmlTag::p1, p1);
 
+            unsigned int texId[3];
+            aiFace face = ReadTriangle(currentNode, texId[0], texId[1], texId[2]);
             if (hasPid && hasP1) {
                 auto it = mResourcesDictionnary.find(pid);
                 if (it != mResourcesDictionnary.end()) {
@@ -420,6 +426,11 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) {
                         mesh->mMaterialIndex = baseMaterials->mMaterialIndex[p1];
                     } else if (it->second->getType() == ResourceType::RT_Texture2DGroup) {
                         if (mesh->mTextureCoords[0] == nullptr) {
+                            mesh->mNumUVComponents[0] = 2;
+                            for (unsigned int i = 1; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) {
+                                mesh->mNumUVComponents[i] = 0;
+                            }
+
                             Texture2DGroup *group = static_cast<Texture2DGroup *>(it->second);
                             const std::string name = ai_to_string(group->mTexId);
                             for (size_t i = 0; i < mMaterials.size(); ++i) {
@@ -427,8 +438,9 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) {
                                     mesh->mMaterialIndex = static_cast<unsigned int>(i);
                                 }
                             }
-                            mesh->mTextureCoords[0] = new aiVector3D[group->mTex2dCoords.size()];
+                            mesh->mTextureCoords[0] = new aiVector3D[mesh->mNumVertices];
-                            for (unsigned int i = 0; i < group->mTex2dCoords.size(); ++i) {
+                            for (unsigned int i = 0; i < mesh->mNumVertices; ++i) {
+
                                 mesh->mTextureCoords[0][i] = aiVector3D(group->mTex2dCoords[i].x, group->mTex2dCoords[i].y, 0);
                             }
                         }
@@ -436,7 +448,6 @@ void XmlSerializer::ImportTriangles(XmlNode &node, aiMesh *mesh) {
                 }
             }
 
-            aiFace face = ReadTriangle(currentNode);
             faces.push_back(face);
         }
     }
@@ -578,11 +589,15 @@ aiMaterial *XmlSerializer::readMaterialDef(XmlNode &node, unsigned int basemater
 }
 
 void XmlSerializer::StoreMaterialsInScene(aiScene *scene) {
-    if (nullptr == scene || mMaterials.empty()) {
+    if (nullptr == scene) {
         return;
     }
 
     scene->mNumMaterials = static_cast<unsigned int>(mMaterials.size());
+    if (scene->mNumMaterials == 0) {
+        return;
+    }
+
     scene->mMaterials = new aiMaterial *[scene->mNumMaterials];
     for (size_t i = 0; i < mMaterials.size(); ++i) {
         scene->mMaterials[i] = mMaterials[i];

code/Common/ScenePreprocessor.cpp

@@ -105,7 +105,9 @@ void ScenePreprocessor::ProcessMesh(aiMesh *mesh) {
     for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) {
         if (!mesh->mTextureCoords[i]) {
             mesh->mNumUVComponents[i] = 0;
-        } else {
+            continue;
+        } 
+
         if (!mesh->mNumUVComponents[i]) {
             mesh->mNumUVComponents[i] = 2;
         }
@@ -116,8 +118,10 @@ void ScenePreprocessor::ProcessMesh(aiMesh *mesh) {
         // as if they were 2D channels .. just in case an application doesn't handle
         // this case
         if (2 == mesh->mNumUVComponents[i]) {
+            size_t num = 0;
             for (; p != end; ++p) {
                 p->z = 0.f;
+                num++;
             }
         } else if (1 == mesh->mNumUVComponents[i]) {
             for (; p != end; ++p) {
@@ -136,7 +140,6 @@ void ScenePreprocessor::ProcessMesh(aiMesh *mesh) {
             }
         }
     }
-    }
 
     // If the information which primitive types are there in the
     // mesh is currently not available, compute it.

include/assimp/XmlParser.h

@@ -99,32 +99,165 @@ template <class TNodeType>
 class TXmlParser {
 public:
     /// @brief The default class constructor.
-    TXmlParser() :
+    TXmlParser();
+
+    ///	@brief  The class destructor.
+    ~TXmlParser();
+
+    ///	@brief  Will clear the parsed xml-file.
+    void clear();
+
+    ///	@brief  Will search for a child-node by its name
+    /// @param  name     [in] The name of the child-node.
+    /// @return The node instance or nullptr, if nothing was found.
+    TNodeType *findNode(const std::string &name);
+
+    /// @brief  Will return true, if the node is a child-node.
+    /// @param  name    [in] The name of the child node to look for.
+    /// @return true, if the node is a child-node or false if not.
+    bool hasNode(const std::string &name);
+
+    /// @brief  Will parse an xml-file from a given stream.
+    /// @param  stream      The input stream.
+    /// @return true, if the parsing was successful, false if not.
+    bool parse(IOStream *stream);
+
+    /// @brief  Will return true if a root node is there.
+    /// @return true in case of an existing root.
+    bool hasRoot() const;
+
+    /// @brief  Will return the document pointer, is nullptr if no xml-file was parsed.
+    /// @return The pointer showing to the document.
+    pugi::xml_document *getDocument() const;
+
+    /// @brief  Will return the root node, const version.
+    /// @return The root node.
+    const TNodeType getRootNode() const;
+
+    /// @brief  Will return the root node, non-const version.
+    /// @return The root node.
+    TNodeType getRootNode();
+
+    /// @brief Will check if a node with the given name is in.
+    /// @param node     [in] The node to look in.
+    /// @param name     [in] The name of the child-node.
+    /// @return true, if node was found, false if not.
+    static inline bool hasNode(XmlNode &node, const char *name);
+
+    /// @brief Will check if an attribute is part of the XmlNode.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in} The attribute name to look for.
+    /// @return true, if the was found, false if not.
+    static inline bool hasAttribute(XmlNode &xmlNode, const char *name);
+
+    /// @brief Will try to get an unsigned int attribute value.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in] The attribute name to look for.
+    /// @param val      [out] The unsigned int value from the attribute.
+    /// @return true, if the node contains an attribute with the given name and if the value is an unsigned int.
+    static inline bool getUIntAttribute(XmlNode &xmlNode, const char *name, unsigned int &val);
+
+    /// @brief Will try to get an int attribute value.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in] The attribute name to look for.
+    /// @param val      [out] The int value from the attribute.
+    /// @return true, if the node contains an attribute with the given name and if the value is an int.
+    static inline bool getIntAttribute(XmlNode &xmlNode, const char *name, int &val);
+
+    /// @brief Will try to get a real attribute value.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in] The attribute name to look for.
+    /// @param val      [out] The real value from the attribute.
+    /// @return true, if the node contains an attribute with the given name and if the value is a real.
+    static inline bool getRealAttribute(XmlNode &xmlNode, const char *name, ai_real &val);
+
+    /// @brief Will try to get a float attribute value.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in] The attribute name to look for.
+    /// @param val      [out] The float value from the attribute.
+    /// @return true, if the node contains an attribute with the given name and if the value is a float.
+    static inline bool getFloatAttribute(XmlNode &xmlNode, const char *name, float &val);
+
+    /// @brief Will try to get a double attribute value.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in] The attribute name to look for.
+    /// @param val      [out] The double value from the attribute.
+    /// @return true, if the node contains an attribute with the given name and if the value is a double.
+    static inline bool getDoubleAttribute(XmlNode &xmlNode, const char *name, double &val);
+
+    /// @brief Will try to get a std::string attribute value.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in] The attribute name to look for.
+    /// @param val      [out] The std::string value from the attribute.
+    /// @return true, if the node contains an attribute with the given name and if the value is a std::string.
+    static inline bool getStdStrAttribute(XmlNode &xmlNode, const char *name, std::string &val);
+
+    /// @brief Will try to get a bool attribute value.
+    /// @param xmlNode  [in] The node to search in.
+    /// @param name     [in] The attribute name to look for.
+    /// @param val      [out] The bool value from the attribute.
+    /// @return true, if the node contains an attribute with the given name and if the value is a bool.
+    static inline bool getBoolAttribute(XmlNode &xmlNode, const char *name, bool &val);
+
+    /// @brief Will try to get the value of the node as a string.
+    /// @param node     [in] The node to search in.
+    /// @param text     [out] The value as a text.
+    /// @return true, if the value can be read out.
+    static inline bool getValueAsString(XmlNode &node, std::string &text);
+
+    /// @brief Will try to get the value of the node as a float.
+    /// @param node     [in] The node to search in.
+    /// @param text     [out] The value as a float.
+    /// @return true, if the value can be read out.
+    static inline bool getValueAsFloat(XmlNode &node, ai_real &v);
+
+    /// @brief Will try to get the value of the node as an integer.
+    /// @param node     [in] The node to search in.
+    /// @param text     [out] The value as a int.
+    /// @return true, if the value can be read out.
+    static inline bool getValueAsInt(XmlNode &node, int &v);
+
+    /// @brief Will try to get the value of the node as an bool.
+    /// @param node     [in] The node to search in.
+    /// @param text     [out] The value as a bool.
+    /// @return true, if the value can be read out.
+    static inline bool getValueAsBool(XmlNode &node, bool &v);
+
+private:
+    pugi::xml_document *mDoc;
+    TNodeType mCurrent;
+    std::vector<char> mData;
+};
+
+template <class TNodeType>
+inline TXmlParser<TNodeType>::TXmlParser() :
         mDoc(nullptr),
         mData() {
     // empty
 }
 
-    ///	@brief  The class destructor.
+template <class TNodeType>
-    ~TXmlParser() {
+inline TXmlParser<TNodeType>::~TXmlParser() {
     clear();
 }
 
-    ///	@brief  Will clear the parsed xml-file.
+template <class TNodeType>
-    void clear() {
+inline void TXmlParser<TNodeType>::clear() {
     if (mData.empty()) {
+        if (mDoc) {
+            delete mDoc;
+        }
         mDoc = nullptr;
         return;
     }
+
     mData.clear();
     delete mDoc;
     mDoc = nullptr;
 }
 
-    ///	@brief  Will search for a child-node by its name
+template <class TNodeType>
-    /// @param  name     [in] The name of the child-node.
+inline TNodeType *TXmlParser<TNodeType>::findNode(const std::string &name) {
-    /// @return The node instance or nullptr, if nothing was found.
-    TNodeType *findNode(const std::string &name) {
     if (name.empty()) {
         return nullptr;
     }
@@ -142,17 +275,17 @@ public:
     return &mCurrent;
 }
 
-    /// @brief  Will return true, if the node is a child-node.
+template <class TNodeType>
-    /// @param  name    [in] The name of the child node to look for.
+bool TXmlParser<TNodeType>::hasNode(const std::string &name) {
-    /// @return true, if the node is a child-node or false if not.
-    bool hasNode(const std::string &name) {
     return nullptr != findNode(name);
 }
 
-    /// @brief  Will parse an xml-file from a given stream.
+template <class TNodeType>
-    /// @param  stream      The input stream.
+bool TXmlParser<TNodeType>::parse(IOStream *stream) {
-    /// @return true, if the parsing was successful, false if not.
+    if (hasRoot()) {
-    bool parse(IOStream *stream) {
+        clear();
+    }
+
     if (nullptr == stream) {
         ASSIMP_LOG_DEBUG("Stream is nullptr.");
         return false;
@@ -174,20 +307,18 @@ public:
     return false;
 }
 
-    /// @brief  Will return truem if a root node is there.
+template <class TNodeType>
-    /// @return true in case of an existing root.
+bool TXmlParser<TNodeType>::hasRoot() const {
-    bool hasRoot() const {
     return nullptr != mDoc;
 }
-    /// @brief  Will return the document pointer, is nullptr if no xml-file was parsed.
+
-    /// @return The pointer showing to the document.
+template <class TNodeType>
-    pugi::xml_document *getDocument() const {
+pugi::xml_document *TXmlParser<TNodeType>::getDocument() const {
     return mDoc;
 }
 
-    /// @brief  Will return the root node, const version.
+template <class TNodeType>
-    /// @return The root node.
+const TNodeType TXmlParser<TNodeType>::getRootNode() const {
-    const TNodeType getRootNode() const {
     static pugi::xml_node none;
     if (nullptr == mDoc) {
         return none;
@@ -195,40 +326,30 @@ public:
     return mDoc->root();
 }
 
-    /// @brief  Will return the root node, non-const version.
+template <class TNodeType>
-    /// @return The root node.
+TNodeType TXmlParser<TNodeType>::getRootNode() {
-    TNodeType getRootNode() {
     static pugi::xml_node none;
     if (nullptr == mDoc) {
         return none;
     }
+
     return mDoc->root();
 }
 
-    /// @brief Will check if a node with the given name is in.
+template <class TNodeType>
-    /// @param node     [in] The node to look in.
+inline bool TXmlParser<TNodeType>::hasNode(XmlNode &node, const char *name) {
-    /// @param name     [in] The name of the child-node.
-    /// @return true, if node was found, false if not.
-    static inline bool hasNode(XmlNode &node, const char *name) {
     pugi::xml_node child = node.find_child(find_node_by_name_predicate(name));
     return !child.empty();
 }
 
-    /// @brief Will check if an attribute is part of the XmlNode.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::hasAttribute(XmlNode &xmlNode, const char *name) {
-    /// @param name     [in} The attribute name to look for.
-    /// @return true, if the was found, false if not.
-    static inline bool hasAttribute(XmlNode &xmlNode, const char *name) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     return !attr.empty();
 }
 
-    /// @brief Will try to get an unsigned int attribute value.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getUIntAttribute(XmlNode &xmlNode, const char *name, unsigned int &val) {
-    /// @param name     [in] The attribute name to look for.
-    /// @param val      [out] The unsigned int value from the attribute.
-    /// @return true, if the node contains an attribute with the given name and if the value is an unsigned int.
-    static inline bool getUIntAttribute(XmlNode &xmlNode, const char *name, unsigned int &val) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     if (attr.empty()) {
         return false;
@@ -238,12 +359,8 @@ public:
     return true;
 }
 
-    /// @brief Will try to get an int attribute value.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getIntAttribute(XmlNode &xmlNode, const char *name, int &val) {
-    /// @param name     [in] The attribute name to look for.
-    /// @param val      [out] The int value from the attribute.
-    /// @return true, if the node contains an attribute with the given name and if the value is an int.
-    static inline bool getIntAttribute(XmlNode &xmlNode, const char *name, int &val) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     if (attr.empty()) {
         return false;
@@ -253,12 +370,8 @@ public:
     return true;
 }
 
-    /// @brief Will try to get a real attribute value.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getRealAttribute(XmlNode &xmlNode, const char *name, ai_real &val) {
-    /// @param name     [in] The attribute name to look for.
-    /// @param val      [out] The real value from the attribute.
-    /// @return true, if the node contains an attribute with the given name and if the value is a real.
-    static inline bool getRealAttribute(XmlNode &xmlNode, const char *name, ai_real &val) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     if (attr.empty()) {
         return false;
@@ -271,71 +384,56 @@ public:
     return true;
 }
 
-    /// @brief Will try to get a float attribute value.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getFloatAttribute(XmlNode &xmlNode, const char *name, float &val) {
-    /// @param name     [in] The attribute name to look for.
-    /// @param val      [out] The float value from the attribute.
-    /// @return true, if the node contains an attribute with the given name and if the value is a float.
-    static inline bool getFloatAttribute(XmlNode &xmlNode, const char *name, float &val) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     if (attr.empty()) {
         return false;
     }
 
     val = attr.as_float();
+
     return true;
 }
 
-    /// @brief Will try to get a double attribute value.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getDoubleAttribute(XmlNode &xmlNode, const char *name, double &val) {
-    /// @param name     [in] The attribute name to look for.
-    /// @param val      [out] The double value from the attribute.
-    /// @return true, if the node contains an attribute with the given name and if the value is a double.
-    static inline bool getDoubleAttribute(XmlNode &xmlNode, const char *name, double &val) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     if (attr.empty()) {
         return false;
     }
 
     val = attr.as_double();
+
     return true;
 }
 
-    /// @brief Will try to get a std::string attribute value.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getStdStrAttribute(XmlNode &xmlNode, const char *name, std::string &val) {
-    /// @param name     [in] The attribute name to look for.
-    /// @param val      [out] The std::string value from the attribute.
-    /// @return true, if the node contains an attribute with the given name and if the value is a std::string.
-    static inline bool getStdStrAttribute(XmlNode &xmlNode, const char *name, std::string &val) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     if (attr.empty()) {
         return false;
     }
 
     val = attr.as_string();
+
     return true;
 }
 
-    /// @brief Will try to get a bool attribute value.
+template <class TNodeType>
-    /// @param xmlNode  [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getBoolAttribute(XmlNode &xmlNode, const char *name, bool &val) {
-    /// @param name     [in] The attribute name to look for.
-    /// @param val      [out] The bool value from the attribute.
-    /// @return true, if the node contains an attribute with the given name and if the value is a bool.
-    static inline bool getBoolAttribute(XmlNode &xmlNode, const char *name, bool &val) {
     pugi::xml_attribute attr = xmlNode.attribute(name);
     if (attr.empty()) {
         return false;
     }
 
     val = attr.as_bool();
+
     return true;
 }
 
-    /// @brief Will try to get the value of the node as a string.
+template <class TNodeType>
-    /// @param node     [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getValueAsString(XmlNode &node, std::string &text) {
-    /// @param text     [out] The value as a text.
-    /// @return true, if the value can be read out.
-    static inline bool getValueAsString(XmlNode &node, std::string &text) {
     text = std::string();
     if (node.empty()) {
         return false;
@@ -346,11 +444,8 @@ public:
     return true;
 }
 
-    /// @brief Will try to get the value of the node as a float.
+template <class TNodeType>
-    /// @param node     [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getValueAsFloat(XmlNode &node, ai_real &v) {
-    /// @param text     [out] The value as a float.
-    /// @return true, if the value can be read out.
-    static inline bool getValueAsFloat(XmlNode &node, ai_real &v) {
     if (node.empty()) {
         return false;
     }
@@ -360,11 +455,8 @@ public:
     return true;
 }
 
-    /// @brief Will try to get the value of the node as an integer.
+template <class TNodeType>
-    /// @param node     [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getValueAsInt(XmlNode &node, int &v) {
-    /// @param text     [out] The value as a int.
-    /// @return true, if the value can be read out.
-    static inline bool getValueAsInt(XmlNode &node, int &v) {
     if (node.empty()) {
         return false;
     }
@@ -374,12 +466,8 @@ public:
     return true;
 }
 
-    /// @brief Will try to get the value of the node as an bool.
+template <class TNodeType>
-    /// @param node     [in] The node to search in.
+inline bool TXmlParser<TNodeType>::getValueAsBool(XmlNode &node, bool &v) {
-    /// @param text     [out] The value as a bool.
-    /// @return true, if the value can be read out.
-    static inline bool getValueAsBool(XmlNode& node, bool& v)
-    {
     if (node.empty()) {
         return false;
     }
@@ -389,12 +477,6 @@ public:
     return true;
 }
 
-private:
-    pugi::xml_document *mDoc;
-    TNodeType mCurrent;
-    std::vector<char> mData;
-};
-
 using XmlParser = TXmlParser<pugi::xml_node>;
 
 ///	@brief  This class declares an iterator to loop through all children of the root node.