From 5f28c51c03dd07ce94d6c41427a61c503ef2567d Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 23 Aug 2022 13:59:42 +0300 Subject: [PATCH 1/7] Apply clang-tidy modernize-loop-convert transformation --- code/AssetLib/SMD/SMDLoader.cpp | 42 +++++++++++++-------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/code/AssetLib/SMD/SMDLoader.cpp b/code/AssetLib/SMD/SMDLoader.cpp index 46fd968c8..f8b760f74 100644 --- a/code/AssetLib/SMD/SMDLoader.cpp +++ b/code/AssetLib/SMD/SMDLoader.cpp @@ -147,10 +147,8 @@ void SMDImporter::InternReadFile( const std::string& pFile, aiScene* scene, IOSy if (!asBones.empty()) { // Check whether all bones have been initialized - for (std::vector::const_iterator - i = asBones.begin(); - i != asBones.end();++i) { - if (!(*i).mName.length()) { + for (const auto &asBone : asBones) { + if (!asBone.mName.length()) { ASSIMP_LOG_WARN("SMD: Not all bones have been initialized"); break; } @@ -210,12 +208,10 @@ void SMDImporter::LogWarning(const char* msg) { void SMDImporter::FixTimeValues() { double dDelta = (double)iSmallestFrame; double dMax = 0.0f; - for (std::vector::iterator - iBone = asBones.begin(); - iBone != asBones.end();++iBone) { + for (auto &asBone : asBones) { for (std::vector::iterator - iKey = (*iBone).sAnim.asKeys.begin(); - iKey != (*iBone).sAnim.asKeys.end();++iKey) { + iKey = asBone.sAnim.asKeys.begin(); + iKey != asBone.sAnim.asKeys.end();++iKey) { (*iKey).dTime -= dDelta; dMax = std::max(dMax, (*iKey).dTime); } @@ -351,8 +347,7 @@ void SMDImporter::CreateOutputMeshes() { if (fSum) { fSum = 1 / fSum; - for (unsigned int iBone = 0;iBone < face.avVertices[iVert].aiBoneLinks.size();++iBone) { - TempWeightListEntry& pairval = face.avVertices[iVert].aiBoneLinks[iBone]; + for (auto &pairval : face.avVertices[iVert].aiBoneLinks) { if (pairval.first >= asBones.size()) { continue; } @@ -411,8 +406,7 @@ void SMDImporter::AddBoneChildren(aiNode* pcNode, uint32_t iParent) { ai_assert( nullptr == pcNode->mChildren); // first count ... - for (unsigned int i = 0; i < asBones.size();++i) { - SMD::Bone& bone = asBones[i]; + for (auto &bone : asBones) { if (bone.iParent == iParent) { ++pcNode->mNumChildren; } @@ -516,21 +510,21 @@ void SMDImporter::CreateOutputAnimation(int index, const std::string &name) { // now build valid keys unsigned int a = 0; - for (std::vector::const_iterator i = asBones.begin(); i != asBones.end(); ++i) { + for (const auto &asBone : asBones) { aiNodeAnim* p = pp[a] = new aiNodeAnim(); // copy the name of the bone - p->mNodeName.Set(i->mName); + p->mNodeName.Set(asBone.mName); - p->mNumRotationKeys = (unsigned int)(*i).sAnim.asKeys.size(); + p->mNumRotationKeys = (unsigned int)asBone.sAnim.asKeys.size(); if (p->mNumRotationKeys){ p->mNumPositionKeys = p->mNumRotationKeys; aiVectorKey* pVecKeys = p->mPositionKeys = new aiVectorKey[p->mNumRotationKeys]; aiQuatKey* pRotKeys = p->mRotationKeys = new aiQuatKey[p->mNumRotationKeys]; for (std::vector::const_iterator - qq = (*i).sAnim.asKeys.begin(); - qq != (*i).sAnim.asKeys.end(); ++qq) { + qq = asBone.sAnim.asKeys.begin(); + qq != asBone.sAnim.asKeys.end(); ++qq) { pRotKeys->mTime = pVecKeys->mTime = (*qq).dTime; // compute the rotation quaternion from the euler angles @@ -982,8 +976,8 @@ void SMDImporter::ParseTriangle(const char* szCurrent, const char** szCurrentOut SkipSpacesAndLineEnd(szCurrent,&szCurrent); // load three vertices - for (unsigned int iVert = 0; iVert < 3;++iVert) { - ParseVertex(szCurrent,&szCurrent, face.avVertices[iVert]); + for (auto &avVertex : face.avVertices) { + ParseVertex(szCurrent,&szCurrent, avVertex); } *szCurrentOut = szCurrent; } @@ -1080,13 +1074,11 @@ void SMDImporter::ParseVertex(const char* szCurrent, } vertex.aiBoneLinks.resize(iSize,std::pair(0,0.0f)); - for (std::vector >::iterator - i = vertex.aiBoneLinks.begin(); - i != vertex.aiBoneLinks.end();++i) { - if(!ParseUnsignedInt(szCurrent,&szCurrent,(*i).first)) { + for (auto &aiBoneLink : vertex.aiBoneLinks) { + if(!ParseUnsignedInt(szCurrent,&szCurrent,aiBoneLink.first)) { SMDI_PARSE_RETURN; } - if(!ParseFloat(szCurrent,&szCurrent,(*i).second)) { + if(!ParseFloat(szCurrent,&szCurrent,aiBoneLink.second)) { SMDI_PARSE_RETURN; } } From 712671e81a51e4f778c42b23362700a6fb4fb85e Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 23 Aug 2022 14:02:36 +0300 Subject: [PATCH 2/7] Apply modernize-loop-convert again --- code/AssetLib/SMD/SMDLoader.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/code/AssetLib/SMD/SMDLoader.cpp b/code/AssetLib/SMD/SMDLoader.cpp index f8b760f74..97637b20a 100644 --- a/code/AssetLib/SMD/SMDLoader.cpp +++ b/code/AssetLib/SMD/SMDLoader.cpp @@ -209,11 +209,9 @@ void SMDImporter::FixTimeValues() { double dDelta = (double)iSmallestFrame; double dMax = 0.0f; for (auto &asBone : asBones) { - for (std::vector::iterator - iKey = asBone.sAnim.asKeys.begin(); - iKey != asBone.sAnim.asKeys.end();++iKey) { - (*iKey).dTime -= dDelta; - dMax = std::max(dMax, (*iKey).dTime); + for (auto &asKey : asBone.sAnim.asKeys) { + asKey.dTime -= dDelta; + dMax = std::max(dMax, asKey.dTime); } } dLengthOfAnim = dMax; @@ -522,15 +520,13 @@ void SMDImporter::CreateOutputAnimation(int index, const std::string &name) { aiVectorKey* pVecKeys = p->mPositionKeys = new aiVectorKey[p->mNumRotationKeys]; aiQuatKey* pRotKeys = p->mRotationKeys = new aiQuatKey[p->mNumRotationKeys]; - for (std::vector::const_iterator - qq = asBone.sAnim.asKeys.begin(); - qq != asBone.sAnim.asKeys.end(); ++qq) { - pRotKeys->mTime = pVecKeys->mTime = (*qq).dTime; + for (const auto &asKey : asBone.sAnim.asKeys) { + pRotKeys->mTime = pVecKeys->mTime = asKey.dTime; // compute the rotation quaternion from the euler angles // aiQuaternion: The order of the parameters is yzx? - pRotKeys->mValue = aiQuaternion((*qq).vRot.y, (*qq).vRot.z, (*qq).vRot.x); - pVecKeys->mValue = (*qq).vPos; + pRotKeys->mValue = aiQuaternion(asKey.vRot.y, asKey.vRot.z, asKey.vRot.x); + pVecKeys->mValue = asKey.vPos; ++pVecKeys; ++pRotKeys; } From 1ca44acebcdc7fae599eb1ae9e038ba4264b1707 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 23 Aug 2022 14:03:48 +0300 Subject: [PATCH 3/7] Simplify a for loop --- code/AssetLib/SMD/SMDLoader.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/AssetLib/SMD/SMDLoader.cpp b/code/AssetLib/SMD/SMDLoader.cpp index 97637b20a..4ff6234c2 100644 --- a/code/AssetLib/SMD/SMDLoader.cpp +++ b/code/AssetLib/SMD/SMDLoader.cpp @@ -244,7 +244,7 @@ void SMDImporter::CreateOutputMeshes() { iNum = 0; for (std::vector::const_iterator iFace = asTriangles.begin(); - iFace != asTriangles.end();++iFace,++iNum) { + iFace != asTriangles.end(); ++iFace) { if (UINT_MAX == (*iFace).iTexture) { aaiFaces[(*iFace).iTexture].push_back( 0 ); } else if ((*iFace).iTexture >= aszTextures.size()) { @@ -253,6 +253,7 @@ void SMDImporter::CreateOutputMeshes() { } else { aaiFaces[(*iFace).iTexture].push_back(iNum); } + ++iNum; } // now create the output meshes From 795c0abcc865f139daf5e44cb89e54bf8b110673 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 23 Aug 2022 14:10:00 +0300 Subject: [PATCH 4/7] Apply modernize-loop-convert to the simplified loop --- code/AssetLib/SMD/SMDLoader.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/code/AssetLib/SMD/SMDLoader.cpp b/code/AssetLib/SMD/SMDLoader.cpp index 4ff6234c2..1a860568a 100644 --- a/code/AssetLib/SMD/SMDLoader.cpp +++ b/code/AssetLib/SMD/SMDLoader.cpp @@ -242,16 +242,14 @@ void SMDImporter::CreateOutputMeshes() { // collect all faces iNum = 0; - for (std::vector::const_iterator - iFace = asTriangles.begin(); - iFace != asTriangles.end(); ++iFace) { - if (UINT_MAX == (*iFace).iTexture) { - aaiFaces[(*iFace).iTexture].push_back( 0 ); - } else if ((*iFace).iTexture >= aszTextures.size()) { + for (const auto &asTriangle : asTriangles) { + if (UINT_MAX == asTriangle.iTexture) { + aaiFaces[asTriangle.iTexture].push_back( 0 ); + } else if (asTriangle.iTexture >= aszTextures.size()) { ASSIMP_LOG_INFO("[SMD/VTA] Material index overflow in face"); - aaiFaces[(*iFace).iTexture].push_back((unsigned int)aszTextures.size()-1); + aaiFaces[asTriangle.iTexture].push_back((unsigned int)aszTextures.size()-1); } else { - aaiFaces[(*iFace).iTexture].push_back(iNum); + aaiFaces[asTriangle.iTexture].push_back(iNum); } ++iNum; } From 03397d42e2dc5fb29d4fc374086daebfe38be21a Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 23 Aug 2022 14:15:06 +0300 Subject: [PATCH 5/7] Use unique_ptr for aaiFaces instead of explicit delete[] --- code/AssetLib/SMD/SMDLoader.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/code/AssetLib/SMD/SMDLoader.cpp b/code/AssetLib/SMD/SMDLoader.cpp index 1a860568a..2a43aab09 100644 --- a/code/AssetLib/SMD/SMDLoader.cpp +++ b/code/AssetLib/SMD/SMDLoader.cpp @@ -231,7 +231,7 @@ void SMDImporter::CreateOutputMeshes() { pScene->mMeshes = new aiMesh*[pScene->mNumMeshes]; typedef std::vector FaceList; - FaceList* aaiFaces = new FaceList[pScene->mNumMeshes]; + std::unique_ptr aaiFaces(new FaceList[pScene->mNumMeshes]); // approximate the space that will be required unsigned int iNum = (unsigned int)asTriangles.size() / pScene->mNumMeshes; @@ -392,7 +392,6 @@ void SMDImporter::CreateOutputMeshes() { } delete[] aaiBones; } - delete[] aaiFaces; } // ------------------------------------------------------------------------------------------------ From 68bc6a06b93ee2881ed4b62dbe7f29294743d33e Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 23 Aug 2022 14:17:56 +0300 Subject: [PATCH 6/7] Use unique_ptr for aaiBones instead of explicit delete[] --- code/AssetLib/SMD/SMDLoader.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/code/AssetLib/SMD/SMDLoader.cpp b/code/AssetLib/SMD/SMDLoader.cpp index 2a43aab09..78ff397b7 100644 --- a/code/AssetLib/SMD/SMDLoader.cpp +++ b/code/AssetLib/SMD/SMDLoader.cpp @@ -268,7 +268,7 @@ void SMDImporter::CreateOutputMeshes() { typedef std::pair TempWeightListEntry; typedef std::vector< TempWeightListEntry > TempBoneWeightList; - TempBoneWeightList* aaiBones = new TempBoneWeightList[asBones.size()](); + std::unique_ptr aaiBones(new TempBoneWeightList[asBones.size()]()); // try to reserve enough memory without wasting too much for (unsigned int iBone = 0; iBone < asBones.size();++iBone) { @@ -390,7 +390,6 @@ void SMDImporter::CreateOutputMeshes() { ++iNum; } } - delete[] aaiBones; } } From f890bc791fc055d25788f9fdeda5fc3e0097924a Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 23 Aug 2022 14:38:27 +0300 Subject: [PATCH 7/7] Fix out of bounds write --- code/AssetLib/SMD/SMDLoader.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/code/AssetLib/SMD/SMDLoader.cpp b/code/AssetLib/SMD/SMDLoader.cpp index 78ff397b7..686851c92 100644 --- a/code/AssetLib/SMD/SMDLoader.cpp +++ b/code/AssetLib/SMD/SMDLoader.cpp @@ -243,9 +243,7 @@ void SMDImporter::CreateOutputMeshes() { // collect all faces iNum = 0; for (const auto &asTriangle : asTriangles) { - if (UINT_MAX == asTriangle.iTexture) { - aaiFaces[asTriangle.iTexture].push_back( 0 ); - } else if (asTriangle.iTexture >= aszTextures.size()) { + if (asTriangle.iTexture >= aszTextures.size()) { ASSIMP_LOG_INFO("[SMD/VTA] Material index overflow in face"); aaiFaces[asTriangle.iTexture].push_back((unsigned int)aszTextures.size()-1); } else {