Merge pull request #2287 from assimp/Coverity-findings

Coverity-findings

code/BlenderCustomData.cpp

@@ -28,7 +28,11 @@ namespace Assimp {
 
 #define IMPL_STRUCT_READ(ty)                                                    \
         bool read##ty(ElemBase *v, const size_t cnt, const FileDatabase &db) {  \
-            return read<ty>(db.dna[#ty], dynamic_cast<ty *>(v), cnt, db);       \
+        ty *ptr = dynamic_cast<ty*>(v);                                         \
+        if (nullptr == ptr) {                                                   \
+            return false;                                                       \
+        }                                                                       \
+        return read<ty>(db.dna[#ty], ptr, cnt, db);                             \
         }
 
 #define IMPL_STRUCT_CREATE(ty)                                                  \

code/ColladaExporter.cpp

@@ -1500,24 +1500,18 @@ void ColladaExporter::WriteNode( const aiScene* pScene, aiNode* pNode)
     // otherwise it is a normal node (NODE)
     const char * node_type;
     bool is_joint, is_skeleton_root = false;
-    if (NULL == findBone(pScene, pNode->mName.C_Str())) {
+    if (nullptr == findBone(pScene, pNode->mName.C_Str())) {
         node_type = "NODE";
         is_joint = false;
     } else {
         node_type = "JOINT";
         is_joint = true;
-        if(!pNode->mParent || NULL == findBone(pScene, pNode->mParent->mName.C_Str()))
+        if (!pNode->mParent || nullptr == findBone(pScene, pNode->mParent->mName.C_Str())) {
             is_skeleton_root = true;
+        }
     }
 
     const std::string node_name_escaped = XMLEscape(pNode->mName.data);
-	/* // customized, Note! the id field is crucial for inter-xml look up, it cannot be replaced with sid ?!
-    mOutput << startstr
-            << "<node ";
-    if(is_skeleton_root)
-        mOutput << "id=\"" << "skeleton_root" << "\" "; // For now, only support one skeleton in a scene.
-    mOutput << (is_joint ? "s" : "") << "id=\"" << node_name_escaped;
-	 */
 	mOutput << startstr << "<node ";
 	if(is_skeleton_root) {
 		mOutput << "id=\"" << node_name_escaped << "\" " << (is_joint ? "sid=\"" + node_name_escaped +"\"" : "") ; // For now, only support one skeleton in a scene.

code/ColladaLoader.cpp

@@ -85,24 +85,27 @@ static const aiImporterDesc desc = {
 // ------------------------------------------------------------------------------------------------
 // Constructor to be privately used by Importer
 ColladaLoader::ColladaLoader()
-    : mFileName()
-	, mMeshIndexByID()
-	, mMaterialIndexByName()
-	, mMeshes()
-	, newMats()
-	, mCameras()
-	, mLights()
-	, mTextures()
-	, mAnims()
-	, noSkeletonMesh( false )
-    , ignoreUpDirection(false)
-    , mNodeNameCounter( 0 )
-{}
+: mFileName()
+, mMeshIndexByID()
+, mMaterialIndexByName()
+, mMeshes()
+, newMats()
+, mCameras()
+, mLights()
+, mTextures()
+, mAnims()
+, noSkeletonMesh( false )
+, ignoreUpDirection(false)
+, useColladaName( false )
+, mNodeNameCounter( 0 ) {
+    // empty
+}
 
 // ------------------------------------------------------------------------------------------------
 // Destructor, private as well
-ColladaLoader::~ColladaLoader()
-{}
+ColladaLoader::~ColladaLoader() {
+    // empty
+}
 
 // ------------------------------------------------------------------------------------------------
 // Returns whether the class can handle the format of the given file.

code/FBXExportNode.h

@@ -70,14 +70,27 @@ public: // public data members
     bool force_has_children = false;
 
 public: // constructors
+    /// The default class constructor.
     Node() = default;
-    Node(const std::string& n) : name(n) {}
+
+    /// The class constructor with the name.
+    Node(const std::string& n)
+    : name(n)
+    , properties()
+    , children()
+    , force_has_children( false ) {
+        // empty
+    }
 
     // convenience template to construct with properties directly
     template <typename... More>
     Node(const std::string& n, const More... more)
-        : name(n)
-        { AddProperties(more...); }
+    : name(n)
+    , properties()
+    , children()
+    , force_has_children(false) {
+        AddProperties(more...);
+    }
 
 public: // functions to add properties or children
     // add a single property to the node

code/FBXExporter.cpp

@@ -131,13 +131,15 @@ namespace Assimp {
 
 } // end of namespace Assimp
 
-FBXExporter::FBXExporter (
-    const aiScene* pScene,
-    const ExportProperties* pProperties
-)
-    : mScene(pScene)
-    , mProperties(pProperties)
-{
+FBXExporter::FBXExporter ( const aiScene* pScene, const ExportProperties* pProperties )
+: binary(false)
+, mScene(pScene)
+, mProperties(pProperties)
+, outfile()
+, connections()
+, mesh_uids()
+, material_uids()
+, node_uids() {
     // will probably need to determine UIDs, connections, etc here.
     // basically anything that needs to be known
     // before we start writing sections to the stream.
@@ -2444,7 +2446,6 @@ void FBXExporter::WriteAnimationCurve(
     // TODO: keyattr flags and data (STUB for now)
     n.AddChild("KeyAttrFlags", std::vector<int32_t>{0});
     n.AddChild("KeyAttrDataFloat", std::vector<float>{0,0,0,0});
-    ai_assert(static_cast<int32_t>(times.size()) <= std::numeric_limits<int32_t>::max());
     n.AddChild(
         "KeyAttrRefCount",
         std::vector<int32_t>{static_cast<int32_t>(times.size())}

code/SceneCombiner.cpp

@@ -738,7 +738,11 @@ void SceneCombiner::MergeBones(aiMesh* out,std::vector<aiMesh*>::const_iterator
 
         // And copy the final weights - adjust the vertex IDs by the
         // face index offset of the corresponding mesh.
-        for (std::vector< BoneSrcIndex >::const_iterator wmit = (*boneIt).pSrcBones.begin(); wmit != wend; ++wmit)  {
+        for (std::vector< BoneSrcIndex >::const_iterator wmit = (*boneIt).pSrcBones.begin(); wmit != (*boneIt).pSrcBones.end(); ++wmit) {
+            if (wmit == wend) {
+                break;
+            }
+
             aiBone* pip = (*wmit).first;
             for (unsigned int mp = 0; mp < pip->mNumWeights;++mp,++avw) {
                 const aiVertexWeight& vfi = pip->mWeights[mp];

code/XGLLoader.cpp

@@ -370,7 +370,7 @@ aiLight* XGLImporter::ReadDirectionalLight()
 // ------------------------------------------------------------------------------------------------
 aiNode* XGLImporter::ReadObject(TempScope& scope, bool skipFirst, const char* closetag)
 {
-    std::unique_ptr<aiNode> nd(new aiNode());
+    aiNode *nd = new aiNode;
     std::vector<aiNode*> children;
     std::vector<unsigned int> meshes;
 
@@ -453,11 +453,11 @@ aiNode* XGLImporter::ReadObject(TempScope& scope, bool skipFirst, const char* cl
         nd->mChildren = new aiNode*[nd->mNumChildren]();
         for(unsigned int i = 0; i < nd->mNumChildren; ++i) {
             nd->mChildren[i] = children[i];
-            children[i]->mParent = nd.get();
+            children[i]->mParent = nd;
         }
     }
 
-    return nd.release();
+    return nd;
 }
 
 // ------------------------------------------------------------------------------------------------
@@ -731,11 +731,10 @@ unsigned int XGLImporter::ResolveMaterialRef(TempScope& scope)
 }
 
 // ------------------------------------------------------------------------------------------------
-void XGLImporter::ReadMaterial(TempScope& scope)
-{
+void XGLImporter::ReadMaterial(TempScope& scope) {
     const unsigned int mat_id = ReadIDAttr();
 
-    std::unique_ptr<aiMaterial> mat(new aiMaterial());
+    aiMaterial *mat(new aiMaterial );
     while (ReadElementUpToClosing("mat"))  {
         const std::string& s = GetElementName();
         if (s == "amb") {
@@ -764,11 +763,10 @@ void XGLImporter::ReadMaterial(TempScope& scope)
         }
     }
 
-    scope.materials[mat_id] = mat.get();
-    scope.materials_linear.push_back(mat.release());
+    scope.materials[mat_id] = mat;
+    scope.materials_linear.push_back(mat);
 }
 
-
 // ----------------------------------------------------------------------------------------------
 void XGLImporter::ReadFaceVertex(const TempMesh& t, TempFace& out)
 {

code/glTF2Asset.h

@@ -656,7 +656,11 @@ namespace glTF2
             } ortographic;
         } cameraProperties;
 
-        Camera() {}
+        Camera()
+        : type(Perspective)
+        , cameraProperties() {
+            // empty
+        }
         void Read(Value& obj, Asset& r);
     };
 

code/glTF2AssetWriter.inl

@@ -744,6 +744,9 @@ namespace glTF2 {
         if (!(dict = FindArray(*container, d.mDictId))) {
             container->AddMember(StringRef(d.mDictId), Value().SetArray().Move(), mDoc.GetAllocator());
             dict = FindArray(*container, d.mDictId);
+            if (nullptr == dict) {
+                return;
+            }
         }
 
         for (size_t i = 0; i < d.mObjs.size(); ++i) {

code/glTF2Exporter.cpp

@@ -643,11 +643,12 @@ void ExportSkin(Asset& mAsset, const aiMesh* aimesh, Ref<Mesh>& meshRef, Ref<Buf
         Ref<Buffer> buf = vertexJointAccessor->bufferView->buffer;
         uint8_t* arrys = new uint8_t[bytesLen];
         unsigned int i = 0;
+        uint8_t* data = new uint8_t[s_bytesPerComp];
         for ( unsigned int j = 0; j <= bytesLen; j += bytesPerComp ){
             size_t len_p = offset + j;
             float f_value = *(float *)&buf->GetPointer()[len_p];
             unsigned short c = static_cast<unsigned short>(f_value);
-            uint8_t* data = new uint8_t[s_bytesPerComp];
+            ::memset(data, 0, s_bytesPerComp * sizeof(uint8_t));
             data = (uint8_t*)&c;
             memcpy(&arrys[i*s_bytesPerComp], data, s_bytesPerComp);
             ++i;
@@ -657,6 +658,8 @@ void ExportSkin(Asset& mAsset, const aiMesh* aimesh, Ref<Mesh>& meshRef, Ref<Buf
         vertexJointAccessor->bufferView->byteLength = s_bytesLen;
 
         p.attributes.joint.push_back( vertexJointAccessor );
+        delete[] arrys;
+        delete[] data;
     }
 
     Ref<Accessor> vertexWeightAccessor = ExportData(mAsset, skinRef->id, bufferRef, aimesh->mNumVertices, vertexWeightData, AttribType::VEC4, AttribType::VEC4, ComponentType_FLOAT);

code/glTF2Importer.cpp

@@ -763,6 +763,12 @@ static void BuildVertexWeightMapping(Mesh::Primitive& primitive, std::vector<std
     }else {
         attr.joint[0]->ExtractData(indices16);
     }
+    // 
+    if (nullptr == indices8 && nullptr == indices16) {
+        // Something went completely wrong!
+        ai_assert(false);
+        return;
+    }
 
     for (int i = 0; i < num_vertices; ++i) {
         for (int j = 0; j < 4; ++j) {

test/unit/utPLYImportExport.cpp

@@ -56,6 +56,9 @@ public:
         const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/PLY/cube.ply", aiProcess_ValidateDataStructure);
         EXPECT_EQ( 1u, scene->mNumMeshes );
         EXPECT_NE( nullptr, scene->mMeshes[0] );
+        if (nullptr == scene->mMeshes[0]) {
+            return false;
+        }
         EXPECT_EQ( 8u, scene->mMeshes[0]->mNumVertices );
         EXPECT_EQ( 6u, scene->mMeshes[0]->mNumFaces );
         

test/unit/utglTF2ImportExport.cpp

@@ -356,11 +356,12 @@ std::vector<char> ReadFile(const char* name) {
     }
 
     ::fseek(p, 0, SEEK_END);
-    const auto size = ::ftell(p);
+    const size_t size = ::ftell(p);
     ::fseek(p, 0, SEEK_SET);
 
     ret.resize(size);
-    ::fread(&ret[0], 1, size, p);
+    const size_t readSize = ::fread(&ret[0], 1, size, p);
+    EXPECT_EQ(readSize, size);
     ::fclose(p);
 
     return ret;