From 05a4fb57f1a71c09bf77f0a885a7f01ae420b9ff Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sat, 15 Nov 2025 12:16:56 +0100 Subject: [PATCH] Reorganize code top prevent security warning. (#6395) * Reorganize code to prevent security warning. --- code/AssetLib/glTF/glTFImporter.cpp | 190 +++++++++++++++------------- code/AssetLib/glTF/glTFImporter.h | 8 +- 2 files changed, 103 insertions(+), 95 deletions(-) diff --git a/code/AssetLib/glTF/glTFImporter.cpp b/code/AssetLib/glTF/glTFImporter.cpp index 901914da1..a5212b164 100644 --- a/code/AssetLib/glTF/glTFImporter.cpp +++ b/code/AssetLib/glTF/glTFImporter.cpp @@ -56,8 +56,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include -#include - using namespace Assimp; using namespace glTF; @@ -78,6 +76,74 @@ static constexpr aiImporterDesc desc = { "gltf glb" }; +namespace { + + void SetMaterialColorProperty(const std::vector &embeddedTexIdxs, Asset &, TexProperty prop, aiMaterial *mat, + aiTextureType texType, const char *pKey, unsigned int type, unsigned int idx) { + if (prop.texture) { + if (prop.texture->source) { + aiString uri(prop.texture->source->uri); + + if (const int texIdx = embeddedTexIdxs[prop.texture->source.GetIndex()]; texIdx != -1) { // embedded + // setup texture reference string (copied from ColladaLoader::FindFilenameForEffectTexture) + uri.data[0] = '*'; + uri.length = 1 + ASSIMP_itoa10(uri.data + 1, AI_MAXLEN - 1, texIdx); + } + + mat->AddProperty(&uri, _AI_MATKEY_TEXTURE_BASE, texType, 0); + } + return; + } + + aiColor4D col; + CopyValue(prop.color, col); + mat->AddProperty(&col, 1, pKey, type, idx); + } + + void SetFace(aiFace &face, int a) { + face.mNumIndices = 1; + face.mIndices = new unsigned int[1]; + face.mIndices[0] = a; + } + + void SetFace(aiFace &face, int a, int b) { + face.mNumIndices = 2; + face.mIndices = new unsigned int[2]; + face.mIndices[0] = a; + face.mIndices[1] = b; + } + + void SetFace(aiFace &face, int a, int b, int c) { + face.mNumIndices = 3; + face.mIndices = new unsigned int[3]; + face.mIndices[0] = a; + face.mIndices[1] = b; + face.mIndices[2] = c; + } + + bool CheckValidFacesIndices(const aiFace *faces, unsigned nFaces, unsigned nVerts) { + for (unsigned i = 0; i < nFaces; ++i) { + for (unsigned j = 0; j < faces[i].mNumIndices; ++j) { + unsigned idx = faces[i].mIndices[j]; + if (idx >= nVerts) + return false; + } + } + return true; + } + + void createDefaultMaterial(aiScene *scene) { + if (scene == nullptr) { + return; + } + scene->mNumMaterials = 1; + scene->mMaterials = nullptr; + scene->mMaterials = new aiMaterial *[1]; + scene->mMaterials[0] = new aiMaterial(); + } + +} // Anonymous namespace + glTFImporter::glTFImporter() : mScene(nullptr) { // empty @@ -99,33 +165,15 @@ bool glTFImporter::CanRead(const std::string &pFile, IOSystem *pIOHandler, bool return false; } } - -inline void SetMaterialColorProperty(const std::vector &embeddedTexIdxs, Asset &, TexProperty prop, aiMaterial *mat, - aiTextureType texType, const char *pKey, unsigned int type, unsigned int idx) { - if (prop.texture) { - if (prop.texture->source) { - aiString uri(prop.texture->source->uri); - - if (const int texIdx = embeddedTexIdxs[prop.texture->source.GetIndex()]; texIdx != -1) { // embedded - // setup texture reference string (copied from ColladaLoader::FindFilenameForEffectTexture) - uri.data[0] = '*'; - uri.length = 1 + ASSIMP_itoa10(uri.data + 1, AI_MAXLEN - 1, texIdx); - } - - mat->AddProperty(&uri, _AI_MATKEY_TEXTURE_BASE, texType, 0); - } + +void glTFImporter::ImportMaterials(Asset &r) const { + mScene->mNumMaterials = r.materials.Size(); + if (mScene->mNumMaterials == 0) { + createDefaultMaterial(mScene); return; } - aiColor4D col; - CopyValue(prop.color, col); - mat->AddProperty(&col, 1, pKey, type, idx); -} - -void glTFImporter::ImportMaterials(Asset &r) { - mScene->mNumMaterials = static_cast(r.materials.Size()); mScene->mMaterials = new aiMaterial *[mScene->mNumMaterials]; - for (unsigned int i = 0; i < mScene->mNumMaterials; ++i) { aiMaterial *aimat = mScene->mMaterials[i] = new aiMaterial(); @@ -148,47 +196,6 @@ void glTFImporter::ImportMaterials(Asset &r) { aimat->AddProperty(&mat.shininess, 1, AI_MATKEY_SHININESS); } } - - if (mScene->mNumMaterials == 0) { - mScene->mNumMaterials = 1; - // Delete the array of length zero created above. - delete[] mScene->mMaterials; - mScene->mMaterials = nullptr; - mScene->mMaterials = new aiMaterial *[1]; - mScene->mMaterials[0] = new aiMaterial(); - } -} - -static void SetFace(aiFace &face, int a) { - face.mNumIndices = 1; - face.mIndices = new unsigned int[1]; - face.mIndices[0] = a; -} - -static void SetFace(aiFace &face, int a, int b) { - face.mNumIndices = 2; - face.mIndices = new unsigned int[2]; - face.mIndices[0] = a; - face.mIndices[1] = b; -} - -static void SetFace(aiFace &face, int a, int b, int c) { - face.mNumIndices = 3; - face.mIndices = new unsigned int[3]; - face.mIndices[0] = a; - face.mIndices[1] = b; - face.mIndices[2] = c; -} - -static bool CheckValidFacesIndices(const aiFace *faces, unsigned nFaces, unsigned nVerts) { - for (unsigned i = 0; i < nFaces; ++i) { - for (unsigned j = 0; j < faces[i].mNumIndices; ++j) { - unsigned idx = faces[i].mIndices[j]; - if (idx >= nVerts) - return false; - } - } - return true; } void glTFImporter::ImportMeshes(Asset &r) { @@ -452,7 +459,7 @@ void glTFImporter::ImportMeshes(Asset &r) { CopyVector(meshes, mScene->mMeshes, mScene->mNumMeshes); } -void glTFImporter::ImportCameras(Asset &r) { +void glTFImporter::ImportCameras(Asset &r) const { if (!r.cameras.Size()) { return; } @@ -460,29 +467,30 @@ void glTFImporter::ImportCameras(Asset &r) { mScene->mNumCameras = r.cameras.Size(); mScene->mCameras = new aiCamera *[r.cameras.Size()]; for (size_t i = 0; i < r.cameras.Size(); ++i) { - Camera &cam = r.cameras[i]; - - aiCamera *aicam = mScene->mCameras[i] = new aiCamera(); - + const Camera &cam = r.cameras[i]; + mScene->mCameras[i] = new aiCamera(); + const auto aiCameraPtr = mScene->mCameras[i]; if (cam.type == Camera::Perspective) { - aicam->mAspect = cam.perspective.aspectRatio; - aicam->mHorizontalFOV = cam.perspective.yfov * ((aicam->mAspect == 0.f) ? 1.f : aicam->mAspect); - aicam->mClipPlaneFar = cam.perspective.zfar; - aicam->mClipPlaneNear = cam.perspective.znear; + aiCameraPtr->mAspect = cam.perspective.aspectRatio; + aiCameraPtr->mHorizontalFOV = cam.perspective.yfov * ((aiCameraPtr->mAspect == 0.f) ? 1.f : aiCameraPtr->mAspect); + aiCameraPtr->mClipPlaneFar = cam.perspective.zfar; + aiCameraPtr->mClipPlaneNear = cam.perspective.znear; } else { - aicam->mClipPlaneFar = cam.ortographic.zfar; - aicam->mClipPlaneNear = cam.ortographic.znear; - aicam->mHorizontalFOV = 0.0; - aicam->mAspect = 1.0f; + aiCameraPtr->mClipPlaneFar = cam.ortographic.zfar; + aiCameraPtr->mClipPlaneNear = cam.ortographic.znear; + aiCameraPtr->mHorizontalFOV = 0.0; + aiCameraPtr->mAspect = 1.0f; if (0.f != cam.ortographic.ymag) { - aicam->mAspect = cam.ortographic.xmag / cam.ortographic.ymag; + aiCameraPtr->mAspect = cam.ortographic.xmag / cam.ortographic.ymag; } } } } -void glTFImporter::ImportLights(Asset &r) { - if (!r.lights.Size()) return; +void glTFImporter::ImportLights(Asset &r) const { + if (!r.lights.Size()) { + return; + } mScene->mNumLights = r.lights.Size(); mScene->mLights = new aiLight *[r.lights.Size()]; @@ -529,7 +537,7 @@ aiNode *ImportNode(aiScene *pScene, Asset &r, std::vector &meshOff aiNode *ainode = new aiNode(node.id); if (!node.children.empty()) { - ainode->mNumChildren = unsigned(node.children.size()); + ainode->mNumChildren = static_cast(node.children.size()); ainode->mChildren = new aiNode *[ainode->mNumChildren]; for (unsigned int i = 0; i < ainode->mNumChildren; ++i) { @@ -578,7 +586,7 @@ aiNode *ImportNode(aiScene *pScene, Asset &r, std::vector &meshOff int k = 0; for (size_t i = 0; i < node.meshes.size(); ++i) { - int idx = node.meshes[i].GetIndex(); + const int idx = node.meshes[i].GetIndex(); for (unsigned int j = meshOffsets[idx]; j < meshOffsets[idx + 1]; ++j, ++k) { ainode->mMeshes[k] = j; } @@ -597,13 +605,14 @@ aiNode *ImportNode(aiScene *pScene, Asset &r, std::vector &meshOff } void glTFImporter::ImportNodes(Asset &r) { - if (!r.scene) return; + if (!r.scene) { + return; + } std::vector> rootNodes = r.scene->nodes; // The root nodes - unsigned int numRootNodes = static_cast(rootNodes.size()); - if (numRootNodes == 1) { // a single root node: use it + if (auto numRootNodes = static_cast(rootNodes.size()); numRootNodes == 1) { // a single root node: use it mScene->mRootNode = ImportNode(mScene, r, meshOffsets, rootNodes[0]); } else if (numRootNodes > 1) { // more than one root node: create a fake root aiNode *root = new aiNode("ROOT"); @@ -648,11 +657,10 @@ void glTFImporter::ImportEmbeddedTextures(Asset &r) { tex->mFilename = img.name; tex->mWidth = static_cast(length); tex->mHeight = 0; - tex->pcData = reinterpret_cast(data); + tex->pcData = static_cast(data); if (!img.mimeType.empty()) { - const char *ext = strchr(img.mimeType.c_str(), '/') + 1; - if (ext) { + if (const char *ext = strchr(img.mimeType.c_str(), '/') + 1) { if (strncmp(ext, "jpeg", 4) == 0) { ext = "jpg"; } @@ -666,7 +674,7 @@ void glTFImporter::ImportEmbeddedTextures(Asset &r) { } } -void glTFImporter::ImportCommonMetadata(Asset &a) { +void glTFImporter::ImportCommonMetadata(const Asset &a) const { ai_assert(mScene->mMetaData == nullptr); const bool hasVersion = !a.asset.version.empty(); diff --git a/code/AssetLib/glTF/glTFImporter.h b/code/AssetLib/glTF/glTFImporter.h index 23082f1d0..42acd7897 100644 --- a/code/AssetLib/glTF/glTFImporter.h +++ b/code/AssetLib/glTF/glTFImporter.h @@ -70,12 +70,12 @@ protected: private: void ImportEmbeddedTextures(glTF::Asset &a); - void ImportMaterials(glTF::Asset &a); + void ImportMaterials(glTF::Asset &a) const; void ImportMeshes(glTF::Asset &a); - void ImportCameras(glTF::Asset &a); - void ImportLights(glTF::Asset &a); + void ImportCameras(glTF::Asset &a) const; + void ImportLights(glTF::Asset &a) const; void ImportNodes(glTF::Asset &a); - void ImportCommonMetadata(glTF::Asset &a); + void ImportCommonMetadata(const glTF::Asset &a) const; private: std::vector meshOffsets;