Browse Source

Fix coverity findings.

Kim Kulling 9 years ago
parent
commit
c5e3058ab3
6 changed files with 75 additions and 50 deletions
  1. 19 17
      code/BaseImporter.cpp
  2. 6 8
      code/X3DImporter_Node.hpp
  3. 11 2
      code/glTFAsset.h
  4. 17 5
      code/glTFAsset.inl
  5. 20 18
      code/glTFExporter.cpp
  6. 2 0
      test/unit/utObjImportExport.cpp

+ 19 - 17
code/BaseImporter.cpp

@@ -56,7 +56,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <sstream>
 #include <sstream>
 #include <cctype>
 #include <cctype>
 
 
-
 using namespace Assimp;
 using namespace Assimp;
 
 
 // ------------------------------------------------------------------------------------------------
 // ------------------------------------------------------------------------------------------------
@@ -461,29 +460,32 @@ void BaseImporter::TextFileToBuffer(IOStream* stream,
 }
 }
 
 
 // ------------------------------------------------------------------------------------------------
 // ------------------------------------------------------------------------------------------------
-namespace Assimp
-{
+namespace Assimp {
     // Represents an import request
     // Represents an import request
-    struct LoadRequest
-    {
+    struct LoadRequest {
         LoadRequest(const std::string& _file, unsigned int _flags,const BatchLoader::PropertyMap* _map, unsigned int _id)
         LoadRequest(const std::string& _file, unsigned int _flags,const BatchLoader::PropertyMap* _map, unsigned int _id)
-            : file(_file), flags(_flags), refCnt(1),scene(NULL), loaded(false), id(_id)
-        {
-            if (_map)
+        : file(_file)
+        , flags(_flags)
+        , refCnt(1)
+        , scene(NULL)
+        , loaded(false)
+        , id(_id) {
+            if ( _map ) {
                 map = *_map;
                 map = *_map;
+            }
         }
         }
 
 
-        const std::string file;
-        unsigned int flags;
-        unsigned int refCnt;
-        aiScene* scene;
-        bool loaded;
-        BatchLoader::PropertyMap map;
-        unsigned int id;
-
-        bool operator== (const std::string& f) const {
+        bool operator== ( const std::string& f ) const {
             return file == f;
             return file == f;
         }
         }
+
+        const std::string        file;
+        unsigned int             flags;
+        unsigned int             refCnt;
+        aiScene                 *scene;
+        bool                     loaded;
+        BatchLoader::PropertyMap map;
+        unsigned int             id;
     };
     };
 }
 }
 
 

+ 6 - 8
code/X3DImporter_Node.hpp

@@ -130,37 +130,35 @@ public:
 public:
 public:
 
 
 	std::string ID;///< ID of the element. Can be empty. In X3D synonym for "ID" attribute.
 	std::string ID;///< ID of the element. Can be empty. In X3D synonym for "ID" attribute.
-	CX3DImporter_NodeElement* Parent;///< Parrent element. If nullptr then this node is root.
+	CX3DImporter_NodeElement* Parent;///< Parent element. If nullptr then this node is root.
 	std::list<CX3DImporter_NodeElement*> Child;///< Child elements.
 	std::list<CX3DImporter_NodeElement*> Child;///< Child elements.
 
 
 	/***********************************************/
 	/***********************************************/
 	/****************** Functions ******************/
 	/****************** Functions ******************/
 	/***********************************************/
 	/***********************************************/
 
 
-private:
+    /// @brief  The destructor, virtual.
+    virtual ~CX3DImporter_NodeElement() {
+        // empty
+    }
 
 
-	/// \fn CX3DImporter_NodeElement(const CX3DImporter_NodeElement& pNodeElement)
+private:
 	/// Disabled copy constructor.
 	/// Disabled copy constructor.
 	CX3DImporter_NodeElement(const CX3DImporter_NodeElement& pNodeElement);
 	CX3DImporter_NodeElement(const CX3DImporter_NodeElement& pNodeElement);
 
 
-	/// \fn CX3DImporter_NodeElement& operator=(const CX3DImporter_NodeElement& pNodeElement)
 	/// Disabled assign operator.
 	/// Disabled assign operator.
 	CX3DImporter_NodeElement& operator=(const CX3DImporter_NodeElement& pNodeElement);
 	CX3DImporter_NodeElement& operator=(const CX3DImporter_NodeElement& pNodeElement);
 
 
-	/// \fn CX3DImporter_NodeElement()
 	/// Disabled default constructor.
 	/// Disabled default constructor.
 	CX3DImporter_NodeElement();
 	CX3DImporter_NodeElement();
 
 
 protected:
 protected:
-
-	/// \fn CX3DImporter_NodeElement(const EType pType, CX3DImporter_NodeElement* pParent)
 	/// In constructor inheritor must set element type.
 	/// In constructor inheritor must set element type.
 	/// \param [in] pType - element type.
 	/// \param [in] pType - element type.
 	/// \param [in] pParent - parent element.
 	/// \param [in] pParent - parent element.
 	CX3DImporter_NodeElement(const EType pType, CX3DImporter_NodeElement* pParent)
 	CX3DImporter_NodeElement(const EType pType, CX3DImporter_NodeElement* pParent)
 		: Type(pType), Parent(pParent)
 		: Type(pType), Parent(pParent)
 	{}
 	{}
-
 };// class IX3DImporter_NodeElement
 };// class IX3DImporter_NodeElement
 
 
 /// \class CX3DImporter_NodeElement_Group
 /// \class CX3DImporter_NodeElement_Group

+ 11 - 2
code/glTFAsset.h

@@ -744,6 +744,10 @@ namespace glTF
 			SExtension(const EType pType)
 			SExtension(const EType pType)
 				: Type(pType)
 				: Type(pType)
 			{}
 			{}
+
+            virtual ~SExtension() {
+                // empty
+            }
 		};
 		};
 
 
 		#ifdef ASSIMP_IMPORTER_GLTF_USE_OPEN3DGC
 		#ifdef ASSIMP_IMPORTER_GLTF_USE_OPEN3DGC
@@ -765,8 +769,13 @@ namespace glTF
 				/// \fn SCompression_Open3DGC
 				/// \fn SCompression_Open3DGC
 				/// Constructor.
 				/// Constructor.
 				SCompression_Open3DGC()
 				SCompression_Open3DGC()
-				: SExtension(Compression_Open3DGC)
-				{}
+				: SExtension(Compression_Open3DGC) {
+                    // empty
+                }
+
+                virtual ~SCompression_Open3DGC() {
+                    // empty
+                }
 			};
 			};
 		#endif
 		#endif
 
 

+ 17 - 5
code/glTFAsset.inl

@@ -1005,6 +1005,12 @@ Ref<Buffer> buf = pAsset_Root.buffers.Get(pCompression_Open3DGC.Buffer);
 		size_t tval = ifs.GetNIntAttribute(idx);
 		size_t tval = ifs.GetNIntAttribute(idx);
 		switch( ifs.GetIntAttributeType( idx ) )
 		switch( ifs.GetIntAttributeType( idx ) )
 		{
 		{
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_UNKOWN:
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_INDEX:
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_JOINT_ID:
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_INDEX_BUFFER_ID:
+                break;
+
 			default:
 			default:
 				throw DeadlyImportError("GLTF: Open3DGC. Unsupported type of int attribute: " + to_string(ifs.GetIntAttributeType(idx)));
 				throw DeadlyImportError("GLTF: Open3DGC. Unsupported type of int attribute: " + to_string(ifs.GetIntAttributeType(idx)));
 		}
 		}
@@ -1049,10 +1055,14 @@ Ref<Buffer> buf = pAsset_Root.buffers.Get(pCompression_Open3DGC.Buffer);
 		}
 		}
 	}
 	}
 
 
-	for(size_t idx = 0, idx_end = size_intattr.size(); idx < idx_end; idx++)
-	{
-		switch(ifs.GetIntAttributeType(idx))
-		{
+	for(size_t idx = 0, idx_end = size_intattr.size(); idx < idx_end; idx++) {
+		switch(ifs.GetIntAttributeType(idx)) {
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_UNKOWN:
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_INDEX:
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_JOINT_ID:
+            case o3dgc::O3DGC_IFS_INT_ATTRIBUTE_TYPE_INDEX_BUFFER_ID:
+                break;
+
 			// ifs.SetIntAttribute(idx, (long* const)(decoded_data + get_buf_offset(primitives[0].attributes.joint)));
 			// ifs.SetIntAttribute(idx, (long* const)(decoded_data + get_buf_offset(primitives[0].attributes.joint)));
 			default:
 			default:
 				throw DeadlyImportError("GLTF: Open3DGC. Unsupported type of int attribute: " + to_string(ifs.GetIntAttributeType(idx)));
 				throw DeadlyImportError("GLTF: Open3DGC. Unsupported type of int attribute: " + to_string(ifs.GetIntAttributeType(idx)));
@@ -1062,7 +1072,9 @@ Ref<Buffer> buf = pAsset_Root.buffers.Get(pCompression_Open3DGC.Buffer);
 	//
 	//
 	// Decode data
 	// Decode data
 	//
 	//
-	if(decoder.DecodePayload(ifs, bstream) != o3dgc::O3DGC_OK) throw DeadlyImportError("GLTF: can not decode Open3DGC data.");
+    if ( decoder.DecodePayload( ifs, bstream ) != o3dgc::O3DGC_OK ) {
+        throw DeadlyImportError( "GLTF: can not decode Open3DGC data." );
+    }
 
 
 	// Set encoded region for "buffer".
 	// Set encoded region for "buffer".
 	buf->EncodedRegion_Mark(pCompression_Open3DGC.Offset, pCompression_Open3DGC.Count, decoded_data, decoded_data_size, id);
 	buf->EncodedRegion_Mark(pCompression_Open3DGC.Offset, pCompression_Open3DGC.Count, decoded_data, decoded_data_size, id);

+ 20 - 18
code/glTFExporter.cpp

@@ -37,9 +37,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
 ----------------------------------------------------------------------
 ----------------------------------------------------------------------
 */
 */
-
-
-
 #ifndef ASSIMP_BUILD_NO_EXPORT
 #ifndef ASSIMP_BUILD_NO_EXPORT
 #ifndef ASSIMP_BUILD_NO_GLTF_EXPORTER
 #ifndef ASSIMP_BUILD_NO_GLTF_EXPORTER
 
 
@@ -94,8 +91,6 @@ namespace Assimp {
 
 
 } // end of namespace Assimp
 } // end of namespace Assimp
 
 
-
-
 glTFExporter::glTFExporter(const char* filename, IOSystem* pIOSystem, const aiScene* pScene,
 glTFExporter::glTFExporter(const char* filename, IOSystem* pIOSystem, const aiScene* pScene,
                            const ExportProperties* pProperties, bool isBinary)
                            const ExportProperties* pProperties, bool isBinary)
     : mFilename(filename)
     : mFilename(filename)
@@ -419,17 +414,18 @@ Ref<Node> FindSkeletonRootJoint(Ref<Skin>& skinRef)
     return parentNodeRef;
     return parentNodeRef;
 }
 }
 
 
-void ExportSkin(Asset& mAsset, const aiMesh* aim, Ref<Mesh>& meshRef, Ref<Buffer>& bufferRef, Ref<Skin>& skinRef, std::vector<aiMatrix4x4>& inverseBindMatricesData)
+void ExportSkin(Asset& mAsset, const aiMesh* aimesh, Ref<Mesh>& meshRef, Ref<Buffer>& bufferRef, Ref<Skin>& skinRef, std::vector<aiMatrix4x4>& inverseBindMatricesData)
 {
 {
-    if (aim->mNumBones < 1) {
+    if (aimesh->mNumBones < 1) {
         return;
         return;
     }
     }
 
 
     // Store the vertex joint and weight data.
     // Store the vertex joint and weight data.
-    vec4* vertexJointData = new vec4[aim->mNumVertices];
-    vec4* vertexWeightData = new vec4[aim->mNumVertices];
-    int* jointsPerVertex = new int[aim->mNumVertices];
-    for (size_t i = 0; i < aim->mNumVertices; ++i) {
+    const size_t NumVerts( aimesh->mNumVertices );
+    vec4* vertexJointData = new vec4[ NumVerts ];
+    vec4* vertexWeightData = new vec4[ NumVerts ];
+    int* jointsPerVertex = new int[ NumVerts ];
+    for (size_t i = 0; i < NumVerts; ++i) {
         jointsPerVertex[i] = 0;
         jointsPerVertex[i] = 0;
         for (size_t j = 0; j < 4; ++j) {
         for (size_t j = 0; j < 4; ++j) {
             vertexJointData[i][j] = 0;
             vertexJointData[i][j] = 0;
@@ -437,8 +433,8 @@ void ExportSkin(Asset& mAsset, const aiMesh* aim, Ref<Mesh>& meshRef, Ref<Buffer
         }
         }
     }
     }
 
 
-    for (unsigned int idx_bone = 0; idx_bone < aim->mNumBones; ++idx_bone) {
-        const aiBone* aib = aim->mBones[idx_bone];
+    for (unsigned int idx_bone = 0; idx_bone < aimesh->mNumBones; ++idx_bone) {
+        const aiBone* aib = aimesh->mBones[idx_bone];
 
 
         // aib->mName   =====>  skinRef->jointNames
         // aib->mName   =====>  skinRef->jointNames
         // Find the node with id = mName.
         // Find the node with id = mName.
@@ -470,7 +466,9 @@ void ExportSkin(Asset& mAsset, const aiMesh* aim, Ref<Mesh>& meshRef, Ref<Buffer
             float vertWeight      = aib->mWeights[idx_weights].mWeight;
             float vertWeight      = aib->mWeights[idx_weights].mWeight;
 
 
             // A vertex can only have at most four joint weights. Ignore all others.
             // A vertex can only have at most four joint weights. Ignore all others.
-            if (jointsPerVertex[vertexId] > 3) { continue; }
+            if (jointsPerVertex[vertexId] > 3) { 
+                continue; 
+            }
 
 
             vertexJointData[vertexId][jointsPerVertex[vertexId]] = jointNamesIndex;
             vertexJointData[vertexId][jointsPerVertex[vertexId]] = jointNamesIndex;
             vertexWeightData[vertexId][jointsPerVertex[vertexId]] = vertWeight;
             vertexWeightData[vertexId][jointsPerVertex[vertexId]] = vertWeight;
@@ -481,11 +479,15 @@ void ExportSkin(Asset& mAsset, const aiMesh* aim, Ref<Mesh>& meshRef, Ref<Buffer
     } // End: for-loop mNumMeshes
     } // End: for-loop mNumMeshes
 
 
     Mesh::Primitive& p = meshRef->primitives.back();
     Mesh::Primitive& p = meshRef->primitives.back();
-    Ref<Accessor> vertexJointAccessor = ExportData(mAsset, skinRef->id, bufferRef, aim->mNumVertices, vertexJointData, AttribType::VEC4, AttribType::VEC4, ComponentType_FLOAT);
-    if (vertexJointAccessor) p.attributes.joint.push_back(vertexJointAccessor);
+    Ref<Accessor> vertexJointAccessor = ExportData(mAsset, skinRef->id, bufferRef, aimesh->mNumVertices, vertexJointData, AttribType::VEC4, AttribType::VEC4, ComponentType_FLOAT);
+    if ( vertexJointAccessor ) {
+        p.attributes.joint.push_back( vertexJointAccessor );
+    }
 
 
-    Ref<Accessor> vertexWeightAccessor = ExportData(mAsset, skinRef->id, bufferRef, aim->mNumVertices, vertexWeightData, AttribType::VEC4, AttribType::VEC4, ComponentType_FLOAT);
-    if (vertexWeightAccessor) p.attributes.weight.push_back(vertexWeightAccessor);
+    Ref<Accessor> vertexWeightAccessor = ExportData(mAsset, skinRef->id, bufferRef, aimesh->mNumVertices, vertexWeightData, AttribType::VEC4, AttribType::VEC4, ComponentType_FLOAT);
+    if ( vertexWeightAccessor ) {
+        p.attributes.weight.push_back( vertexWeightAccessor );
+    }
 }
 }
 
 
 void glTFExporter::ExportMeshes()
 void glTFExporter::ExportMeshes()

+ 2 - 0
test/unit/utObjImportExport.cpp

@@ -186,4 +186,6 @@ TEST_F( utObjImportExport, obj_import_test ) {
     SceneDiffer differ;
     SceneDiffer differ;
     EXPECT_TRUE( differ.isEqual( expected, scene ) );
     EXPECT_TRUE( differ.isEqual( expected, scene ) );
     differ.showReport();
     differ.showReport();
+
+    m_im->FreeScene();
 }
 }