Merge pull request #2397 from Cgettys/fix-more-lgtm-alerts

Fix more LGTM Alerts

code/AssxmlExporter.cpp

@@ -555,8 +555,6 @@ void WriteDump(const aiScene* scene, IOStream* io, bool shortened) {
                             mesh->mNormals[n].z);
                     }
                 }
-                else {
-                }
                 ioprintf(io,"\t\t</Normals>\n");
             }
 

code/BlenderDNA.h

@@ -416,7 +416,7 @@ template <> struct Structure :: _defaultInitializer<ErrorPolicy_Fail> {
     void operator ()(T& /*out*/,const char* = "") {
         // obviously, it is crucial that _DefaultInitializer is used
         // only from within a catch clause.
-        throw;
+        throw DeadlyImportError("Constructing BlenderDNA Structure encountered an error");
     }
 };
 

code/COBLoader.cpp

@@ -144,7 +144,7 @@ void COBImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOS
     // check header
     char head[32];
     stream->CopyAndAdvance(head,32);
-    if (strncmp(head,"Caligari ",9)) {
+    if (strncmp(head,"Caligari ",9) != 0) {
         ThrowException("Could not found magic id: `Caligari`");
     }
 
@@ -656,14 +656,14 @@ void COBImporter::ReadLght_Ascii(Scene& out, LineSplitter& splitter, const Chunk
     ReadFloat3Tuple_Ascii(msh.color ,&rgb);
 
     SkipSpaces(&rgb);
-    if (strncmp(rgb,"cone angle",10)) {
+    if (strncmp(rgb,"cone angle",10) != 0) {
         ASSIMP_LOG_WARN_F( "Expected `cone angle` entity in `color` line in `Lght` chunk ", nfo.id );
     }
     SkipSpaces(rgb+10,&rgb);
     msh.angle = fast_atof(&rgb);
 
     SkipSpaces(&rgb);
-    if (strncmp(rgb,"inner angle",11)) {
+    if (strncmp(rgb,"inner angle",11) != 0) {
         ASSIMP_LOG_WARN_F( "Expected `inner angle` entity in `color` line in `Lght` chunk ", nfo.id);
     }
     SkipSpaces(rgb+11,&rgb);
@@ -903,7 +903,7 @@ public:
         if(nfo.size != static_cast<unsigned int>(-1)) {
             try {
                 reader.IncPtr( static_cast< int >( nfo.size ) - reader.GetCurrentPos() + cur );
-            } catch ( DeadlyImportError e ) {
+            } catch (const DeadlyImportError& e ) {
                 // out of limit so correct the value
                 reader.IncPtr( reader.GetReadLimit() );
             }
@@ -1214,7 +1214,7 @@ void COBImporter::ReadGrou_Binary(COB::Scene& out, StreamReaderLE& reader, const
 
     const chunk_guard cn(nfo,reader);
 
-    out.nodes.push_back(std::shared_ptr<Group>(new Group()));
+    out.nodes.push_back(std::make_shared<Group>());
     Group& msh = (Group&)(*out.nodes.back().get());
     msh = nfo;
 

code/D3MFOpcPackage.cpp

@@ -476,8 +476,11 @@ D3MFOpcPackage::D3MFOpcPackage(IOSystem* pIOHandler, const std::string& rFile)
             mZipArchive->Close( fileStream );
 
         } else if( file == D3MF::XmlTag::CONTENT_TYPES_ARCHIVE) {
-
+            ASSIMP_LOG_WARN_F("Ignored file of unsupported type CONTENT_TYPES_ARCHIVES",file);
+        } else {
+            ASSIMP_LOG_WARN_F("Ignored file of unknown type: ",file);
         }
+
     }
 }
 

code/FBXConverter.cpp

@@ -1712,22 +1712,22 @@ namespace Assimp {
                         if (!mesh)
                         {
                             for (const MeshMap::value_type& v : meshes_converted) {
-                                const MeshGeometry* const mesh = dynamic_cast<const MeshGeometry*> (v.first);
+                                const MeshGeometry* const meshGeom = dynamic_cast<const MeshGeometry*> (v.first);
-                                if (!mesh) {
+                                if (!meshGeom) {
                                     continue;
                                 }
 
-                                const MatIndexArray& mats = mesh->GetMaterialIndices();
+                                const MatIndexArray& mats = meshGeom->GetMaterialIndices();
                                 if (std::find(mats.begin(), mats.end(), matIndex) == mats.end()) {
                                     continue;
                                 }
 
                                 int index = -1;
                                 for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) {
-                                    if (mesh->GetTextureCoords(i).empty()) {
+                                    if (meshGeom->GetTextureCoords(i).empty()) {
                                         break;
                                     }
-                                    const std::string& name = mesh->GetTextureCoordChannelName(i);
+                                    const std::string& name = meshGeom->GetTextureCoordChannelName(i);
                                     if (name == uvSet) {
                                         index = static_cast<int>(i);
                                         break;
@@ -1835,22 +1835,22 @@ namespace Assimp {
                         if (!mesh)
                         {
                             for (const MeshMap::value_type& v : meshes_converted) {
-                                const MeshGeometry* const mesh = dynamic_cast<const MeshGeometry*> (v.first);
+                                const MeshGeometry* const meshGeom = dynamic_cast<const MeshGeometry*> (v.first);
-                                if (!mesh) {
+                                if (!meshGeom) {
                                     continue;
                                 }
 
-                                const MatIndexArray& mats = mesh->GetMaterialIndices();
+                                const MatIndexArray& mats = meshGeom->GetMaterialIndices();
                                 if (std::find(mats.begin(), mats.end(), matIndex) == mats.end()) {
                                     continue;
                                 }
 
                                 int index = -1;
                                 for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) {
-                                    if (mesh->GetTextureCoords(i).empty()) {
+                                    if (meshGeom->GetTextureCoords(i).empty()) {
                                         break;
                                     }
-                                    const std::string& name = mesh->GetTextureCoordChannelName(i);
+                                    const std::string& name = meshGeom->GetTextureCoordChannelName(i);
                                     if (name == uvSet) {
                                         index = static_cast<int>(i);
                                         break;
@@ -2191,22 +2191,22 @@ void FBXConverter::SetShadingPropertiesRaw(aiMaterial* out_mat, const PropertyTa
                     if (!mesh)
                     {
                         for (const MeshMap::value_type& v : meshes_converted) {
-                            const MeshGeometry* const mesh = dynamic_cast<const MeshGeometry*>(v.first);
+                            const MeshGeometry* const meshGeom = dynamic_cast<const MeshGeometry*>(v.first);
-                            if (!mesh) {
+                            if (!meshGeom) {
                                 continue;
                             }
 
-                            const MatIndexArray& mats = mesh->GetMaterialIndices();
+                            const MatIndexArray& mats = meshGeom->GetMaterialIndices();
                             if (std::find(mats.begin(), mats.end(), matIndex) == mats.end()) {
                                 continue;
                             }
 
                             int index = -1;
                             for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) {
-                                if (mesh->GetTextureCoords(i).empty()) {
+                                if (meshGeom->GetTextureCoords(i).empty()) {
                                     break;
                                 }
-                                const std::string& name = mesh->GetTextureCoordChannelName(i);
+                                const std::string& name = meshGeom->GetTextureCoordChannelName(i);
                                 if (name == uvSet) {
                                     index = static_cast<int>(i);
                                     break;

code/FBXMaterial.cpp

@@ -326,8 +326,11 @@ Video::Video(uint64_t id, const Element& element, const Document& doc, const std
                 content = new uint8_t[len];
                 ::memcpy(content, data + 5, len);
             }
-        } catch (runtime_error runtimeError) {
+        } catch (const runtime_error& runtimeError)
+        {
             //we don't need the content data for contents that has already been loaded
+            ASSIMP_LOG_DEBUG_F("Caught exception in FBXMaterial (likely because content was already loaded): ",
+                    runtimeError.what());
         }
     }
 

code/IRRLoader.cpp

@@ -304,9 +304,6 @@ inline void FindSuitableMultiple(int& angle)
     else if (angle < 10) angle = 10;
     else if (angle < 20) angle = 20;
     else if (angle < 30) angle = 30;
-    else
-    {
-    }
 }
 
 // ------------------------------------------------------------------------------------------------
@@ -317,6 +314,8 @@ void IRRImporter::ComputeAnimations(Node* root, aiNode* real, std::vector<aiNode
     // XXX totally WIP - doesn't produce proper results, need to evaluate
     // whether there's any use for Irrlicht's proprietary scene format
     // outside Irrlicht ...
+    // This also applies to the above function of FindSuitableMultiple and ClampSpline which are
+    // solely used in this function
 
     if (root->animators.empty()) {
         return;
@@ -674,38 +673,38 @@ void IRRImporter::GenerateGraph(Node* root,aiNode* rootOut ,aiScene* scene,
             // Get the loaded mesh from the scene and add it to
             // the list of all scenes to be attached to the
             // graph we're currently building
-            aiScene* scene = batch.GetImport(root->id);
+            aiScene* localScene = batch.GetImport(root->id);
-            if (!scene) {
+            if (!localScene) {
                 ASSIMP_LOG_ERROR("IRR: Unable to load external file: " + root->meshPath);
                 break;
             }
-            attach.push_back(AttachmentInfo(scene,rootOut));
+            attach.push_back(AttachmentInfo(localScene,rootOut));
 
             // Now combine the material we've loaded for this mesh
             // with the real materials we got from the file. As we
             // don't execute any pp-steps on the file, the numbers
             // should be equal. If they are not, we can impossibly
             // do this  ...
-            if (root->materials.size() != (unsigned int)scene->mNumMaterials)   {
+            if (root->materials.size() != (unsigned int)localScene->mNumMaterials)   {
                 ASSIMP_LOG_WARN("IRR: Failed to match imported materials "
                     "with the materials found in the IRR scene file");
 
                 break;
             }
-            for (unsigned int i = 0; i < scene->mNumMaterials;++i)  {
+            for (unsigned int i = 0; i < localScene->mNumMaterials;++i)  {
                 // Delete the old material, we don't need it anymore
-                delete scene->mMaterials[i];
+                delete localScene->mMaterials[i];
 
                 std::pair<aiMaterial*, unsigned int>& src = root->materials[i];
-                scene->mMaterials[i] = src.first;
+                localScene->mMaterials[i] = src.first;
             }
 
             // NOTE: Each mesh should have exactly one material assigned,
             // but we do it in a separate loop if this behaviour changes
             // in future.
-            for (unsigned int i = 0; i < scene->mNumMeshes;++i) {
+            for (unsigned int i = 0; i < localScene->mNumMeshes;++i) {
                 // Process material flags
-                aiMesh* mesh = scene->mMeshes[i];
+                aiMesh* mesh = localScene->mMeshes[i];
 
 
                 // If "trans_vertex_alpha" mode is enabled, search all vertex colors

code/Importer/STEPParser/STEPFileReader.cpp

@@ -278,10 +278,10 @@ void STEP::ReadFile(DB& db,const EXPRESS::ConversionSchema& scheme,
         std::transform( type.begin(), type.end(), type.begin(), &Assimp::ToLower<char>  );
         const char* sz = scheme.GetStaticStringForToken(type);
         if(sz) {
-            const std::string::size_type len = n2-n1+1;
+            const std::string::size_type szLen = n2-n1+1;
-            char* const copysz = new char[len+1];
+            char* const copysz = new char[szLen+1];
             std::copy(s.c_str()+n1,s.c_str()+n2+1,copysz);
-            copysz[len] = '\0';
+            copysz[szLen] = '\0';
             db.InternInsert(new LazyObject(db,id,line,sz,copysz));
         }
         if(!has_next) {

code/MD5Loader.cpp

@@ -443,10 +443,10 @@ void MD5Importer::LoadMD5MeshFile ()
         for (MD5::VertexList::const_iterator iter =  meshSrc.mVertices.begin();iter != meshSrc.mVertices.end();++iter,++pv) {
             for (unsigned int jub = (*iter).mFirstWeight, w = jub; w < jub + (*iter).mNumWeights;++w)
             {
-                MD5::WeightDesc& desc = meshSrc.mWeights[w];
+                MD5::WeightDesc& weightDesc = meshSrc.mWeights[w];
                 /* FIX for some invalid exporters */
-                if (!(desc.mWeight < AI_MD5_WEIGHT_EPSILON && desc.mWeight >= -AI_MD5_WEIGHT_EPSILON ))
+                if (!(weightDesc.mWeight < AI_MD5_WEIGHT_EPSILON && weightDesc.mWeight >= -AI_MD5_WEIGHT_EPSILON ))
-                    ++piCount[desc.mBone];
+                    ++piCount[weightDesc.mBone];
             }
         }
 
@@ -493,20 +493,20 @@ void MD5Importer::LoadMD5MeshFile ()
                     if (w >= meshSrc.mWeights.size())
                         throw DeadlyImportError("MD5MESH: Invalid weight index");
 
-                    MD5::WeightDesc& desc = meshSrc.mWeights[w];
+                    MD5::WeightDesc& weightDesc = meshSrc.mWeights[w];
-                    if ( desc.mWeight < AI_MD5_WEIGHT_EPSILON && desc.mWeight >= -AI_MD5_WEIGHT_EPSILON) {
+                    if ( weightDesc.mWeight < AI_MD5_WEIGHT_EPSILON && weightDesc.mWeight >= -AI_MD5_WEIGHT_EPSILON) {
                         continue;
                     }
 
-                    const ai_real fNewWeight = desc.mWeight / fSum;
+                    const ai_real fNewWeight = weightDesc.mWeight / fSum;
 
                     // transform the local position into worldspace
-                    MD5::BoneDesc& boneSrc = meshParser.mJoints[desc.mBone];
+                    MD5::BoneDesc& boneSrc = meshParser.mJoints[weightDesc.mBone];
-                    const aiVector3D v = boneSrc.mRotationQuatConverted.Rotate (desc.vOffsetPosition);
+                    const aiVector3D v = boneSrc.mRotationQuatConverted.Rotate (weightDesc.vOffsetPosition);
 
                     // use the original weight to compute the vertex position
                     // (some MD5s seem to depend on the invalid weight values ...)
-                    *pv += ((boneSrc.mPositionXYZ+v)* (ai_real)desc.mWeight);
+                    *pv += ((boneSrc.mPositionXYZ+v)* (ai_real)weightDesc.mWeight);
 
                     aiBone* bone = mesh->mBones[boneSrc.mMap];
                     *bone->mWeights++ = aiVertexWeight((unsigned int)(pv-mesh->mVertices),fNewWeight);

code/STLExporter.cpp

@@ -127,7 +127,7 @@ STLExporter::STLExporter(const char* _filename, const aiScene* pScene, bool expo
         mOutput.write((char *)&meshnum, 4);
 
         if (exportPointClouds) {
-
+            throw DeadlyExportError("This functionality is not yet implemented for binary output.");
         }
 
         for(unsigned int i = 0; i < pScene->mNumMeshes; ++i) {

code/glTFExporter.cpp

@@ -245,7 +245,7 @@ inline Ref<Accessor> ExportData(Asset& a, std::string& meshName, Ref<Buffer>& bu
 
 namespace {
     void GetMatScalar(const aiMaterial* mat, float& val, const char* propName, int type, int idx) {
-        if (mat->Get(propName, type, idx, val) == AI_SUCCESS) {}
+        ai_assert(mat->Get(propName, type, idx, val) == AI_SUCCESS);
     }
 }
 

tools/assimp_cmd/ImageExtractor.cpp

@@ -229,6 +229,7 @@ int DoExport(const aiTexture* tx, FILE* p, const std::string& extension,
 int Assimp_Extract (const char* const* params, unsigned int num)
 {
     const char* const invalid = "assimp extract: Invalid number of arguments. See \'assimp extract --help\'\n";
+    // assimp extract in out [options]
     if (num < 1) {
         printf(invalid);
         return 1;
@@ -240,11 +241,7 @@ int Assimp_Extract (const char* const* params, unsigned int num)
         return 0;
     }
 
-    // asssimp extract in out [options]
+
-    if (num < 1) {
-        printf(invalid);
-        return 1;
-    }
 
     std::string in  = std::string(params[0]);
     std::string out = (num > 1 ? std::string(params[1]) : "-");