bugfix:

- fix redundant checks against nullptr ( see https://github.com/assimp/assimp/issues/241 ).

Signed-off-by: Kim Kulling <[email protected]>

code/Q3BSPFileData.h

@@ -42,10 +42,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <vector>
 
-namespace Assimp
-{
-namespace Q3BSP
-{
+namespace Assimp {
+namespace Q3BSP {
 
 static const unsigned int CE_BSP_LIGHTMAPWIDTH = 128;
 static const unsigned int CE_BSP_LIGHTMAPHEIGHT = 128;
@@ -54,8 +52,7 @@ static const unsigned int CE_BSP_LIGHTMAPSIZE = 128*128*3;	///< = 128( width ) *
 static const int VERION_Q3LEVEL = 46;						///< Supported version.
 
 ///	Geometric type enumeration
-enum Q3BSPGeoType
-{
+enum Q3BSPGeoType {
 	Polygon = 1,
 	Patch, 
 	TriangleMesh,
@@ -63,25 +60,23 @@ enum Q3BSPGeoType
 };
 
 ///	Integer vector.
-struct ceVec3i 
-{
+struct ceVec3i {
     int x, y, z;
 	ceVec3i(): x( 0 ), y( 0 ), z( 0 ) { /* empty */ }
 	ceVec3i( int iX, int iY=0, int iZ=0) : x( iX ), y( iY ), z( iZ ) { /* empty */ }
 };
 
-///	Fileheader
-struct sQ3BSPHeader 
-{
-	char strID[ 4 ];	//!< Should be "IBSP"
-	int iVersion;	//!< 46 for standard levels
+///	the file header
+struct sQ3BSPHeader {
+	char strID[ 4 ]; ///< Should be "IBSP"
+    int iVersion;    ///< 46 for standard levels
 };
 
-///	Descripes an entry.
+///	Describes an entry.
 struct sQ3BSPLump 
 {
-	int iOffset;	///< Offset from startpointer of file
-	int iSize;		///< Size fo part
+	int iOffset;	///< Offset from start pointer of file
+	int iSize;		///< Size of part
 };
 
 struct vec2f
@@ -108,47 +103,42 @@ struct sQ3BSPVertex
 struct sQ3BSPFace 
 {
 	int iTextureID;					///< Index in texture array
-	int iEffect;					///< Index in effectarray (-1 = no effect)
+	int iEffect;					///< Index in effect array (-1 = no effect)
 	int iType;						///< 1=Polygon, 2=Patch, 3=Mesh, 4=Billboard
 	int iVertexIndex;				///< Start index of polygon
 	int iNumOfVerts;				///< Number of vertices
 	int	iFaceVertexIndex;			///< Index of first mesh vertex
-	int iNumOfFaceVerts;			///< Anzahl der Meshvertices
-	int iLightmapID;				///< Index to the lightmap array
-	int iLMapCorner[ 2 ];			///< Die Ecke der Lightmap in der Textur
-	int iLMapSize[ 2 ];				///< Size of the lightmap stored on the texture
-	vec3f vLMapPos;					///< 3D-Ursprung der Lightmap
-	vec3f vLMapVecs[ 2 ];			///< 3D-s-t-Vektoren
-	vec3f vNormal;					///< Polygonnormale
+	int iNumOfFaceVerts;			///< number of mesh vertices
+	int iLightmapID;				///< Index to the light-map array
+	int iLMapCorner[ 2 ];			///< edge of the light-map in texture
+	int iLMapSize[ 2 ];				///< Size of the light-map stored on the texture
+	vec3f vLMapPos;					///< 3D origin of the light-map
+	vec3f vLMapVecs[ 2 ];			///< 3D-s-t-vectors
+	vec3f vNormal;					///< Polygon normals
 	int patchWidth, patchHeight;	///< bezier patch
 };
 
 /// A quake3 texture name.
-struct sQ3BSPTexture 
-{
-	char strName[ 64 ];		///< Name of the texture without extention
+struct sQ3BSPTexture {
+	char strName[ 64 ];		///< Name of the texture without extension
 	int iFlags;				///< Not used
 	int iContents;			///< Not used
 };
 
-///	A lightmap of the level, size 128 x 128, RGB components.
-struct sQ3BSPLightmap 
-{
+///	A light-map of the level, size 128 x 128, RGB components.
+struct sQ3BSPLightmap {
 	unsigned char bLMapData[ CE_BSP_LIGHTMAPSIZE ];
-	sQ3BSPLightmap() 
-	{	
-		memset(bLMapData, 0, CE_BSP_LIGHTMAPSIZE ); 
+	sQ3BSPLightmap() {	
+		::memset(bLMapData, 0, CE_BSP_LIGHTMAPSIZE ); 
 	}
 };
 
-struct SubPatch
-{
+struct SubPatch {
 	std::vector<size_t> indices;
 	int lightmapID;
 };
 
-enum eLumps 
-{
+enum eLumps {
 	kEntities = 0,
 	kTextures,
 	kPlanes,
@@ -169,8 +159,7 @@ enum eLumps
 	kMaxLumps
 };
 
-struct Q3BSPModel
-{
+struct Q3BSPModel {
 	std::vector<unsigned char> m_Data;
 	std::vector<sQ3BSPLump*> m_Lumps;
 	std::vector<sQ3BSPVertex*> m_Vertices;
@@ -195,24 +184,22 @@ struct Q3BSPModel
 		// empty
 	}
 
-	~Q3BSPModel()
-	{
-		for ( unsigned int i=0; i<m_Lumps.size(); i++ )
-			if ( NULL != m_Lumps[i] )
-				delete m_Lumps[i];
-		
-		for ( unsigned int i=0; i<m_Vertices.size(); i++ )
-			if ( NULL != m_Vertices[ i ] )
-				delete m_Vertices[ i ];
-		for ( unsigned int i=0; i<m_Faces.size(); i++ )
-			if ( NULL != m_Faces[ i ] )
-				delete m_Faces[ i ];
-		for ( unsigned int i=0; i<m_Textures.size(); i++ )
-			if ( NULL != m_Textures[ i ] )
-				delete m_Textures[ i ];
-		for ( unsigned int i=0; i<m_Lightmaps.size(); i++ )
-			if ( NULL != m_Lightmaps[ i ] )
-				delete m_Lightmaps[ i ];
+	~Q3BSPModel() {
+		for ( unsigned int i=0; i<m_Lumps.size(); i++ ) {
+            delete m_Lumps[ i ];
+        }
+		for ( unsigned int i=0; i<m_Vertices.size(); i++ ) {
+            delete m_Vertices[ i ];
+        }
+		for ( unsigned int i=0; i<m_Faces.size(); i++ ) {
+            delete m_Faces[ i ];
+        }
+		for ( unsigned int i=0; i<m_Textures.size(); i++ ) {
+            delete m_Textures[ i ];
+        }
+		for ( unsigned int i=0; i<m_Lightmaps.size(); i++ ) {
+            delete m_Lightmaps[ i ];
+        }
 
 		m_Lumps.clear();
 		m_Vertices.clear();

code/Q3BSPFileImporter.cpp

@@ -71,9 +71,14 @@ static const aiImporterDesc desc = {
 	"pk3"
 };
 
-namespace Assimp
-{
+namespace Assimp {
 
+static void getSupportedExtensions(std::vector<std::string> &supportedExtensions) {
+    supportedExtensions.push_back( ".jpg" );
+    supportedExtensions.push_back( ".png" );
+    supportedExtensions.push_back( ".tga" );
+}
+    
 using namespace Q3BSP;
 
 // ------------------------------------------------------------------------------------------------
@@ -86,7 +91,7 @@ static void createKey( int id1, int id2, std::string &rKey )
 }
 
 // ------------------------------------------------------------------------------------------------
-//	Local function to extract the texture ids from a material keyname.
+//	Local function to extract the texture ids from a material key-name.
 static void extractIds( const std::string &rKey, int &rId1, int &rId2 )
 {
 	rId1 = -1;
@@ -146,24 +151,16 @@ Q3BSPFileImporter::Q3BSPFileImporter() :
 
 // ------------------------------------------------------------------------------------------------
 //	Destructor.
-Q3BSPFileImporter::~Q3BSPFileImporter()
-{
-	// For lint
+Q3BSPFileImporter::~Q3BSPFileImporter() {
 	m_pCurrentMesh = NULL;
 	m_pCurrentFace = NULL;
 	
 	// Clear face-to-material map
-	for ( FaceMap::iterator it = m_MaterialLookupMap.begin(); it != m_MaterialLookupMap.end();
-		++it )
-	{
-		const std::string matName = (*it).first;
-		if ( matName.empty() )
-		{
-			continue;
+    for ( FaceMap::iterator it = m_MaterialLookupMap.begin(); it != m_MaterialLookupMap.end(); ++it ) {
+		const std::string &matName = it->first;
+		if ( !matName.empty() ) {
+            delete it->second;
 		}
-
-		std::vector<Q3BSP::sQ3BSPFace*> *pCurFaceArray = (*it).second;
-		delete pCurFaceArray;
 	}
 	m_MaterialLookupMap.clear();
 }
@@ -566,7 +563,7 @@ size_t Q3BSPFileImporter::countFaces( const std::vector<Q3BSP::sQ3BSPFace*> &rAr
 }
 
 // ------------------------------------------------------------------------------------------------
-//	Counts the number of triangles in a Q3-facearray.
+//	Counts the number of triangles in a Q3-face-array.
 size_t Q3BSPFileImporter::countTriangles( const std::vector<Q3BSP::sQ3BSPFace*> &rArray ) const
 {
 	size_t numTriangles = 0;
@@ -617,16 +614,11 @@ void Q3BSPFileImporter::createMaterialMap( const Q3BSP::Q3BSPModel *pModel )
 //	Returns the next face.
 aiFace *Q3BSPFileImporter::getNextFace( aiMesh *pMesh, unsigned int &rFaceIdx )
 {
-	aiFace *pFace = NULL;
-	if ( rFaceIdx < pMesh->mNumFaces )
-	{
+	aiFace *pFace( NULL );
+	if ( rFaceIdx < pMesh->mNumFaces ) {
 		pFace = &pMesh->mFaces[ rFaceIdx ];
 		rFaceIdx++;
 	}
-	else
-	{
-		pFace = NULL;
-	}
 
 	return pFace;
 }
@@ -634,34 +626,30 @@ aiFace *Q3BSPFileImporter::getNextFace( aiMesh *pMesh, unsigned int &rFaceIdx )
 // ------------------------------------------------------------------------------------------------
 //	Imports a texture file.
 bool Q3BSPFileImporter::importTextureFromArchive( const Q3BSP::Q3BSPModel *pModel,
-												 Q3BSP::Q3BSPZipArchive *pArchive, aiScene* /*pScene*/,
-												 aiMaterial *pMatHelper, int textureId )
-{
-	std::vector<std::string> supportedExtensions;
-	supportedExtensions.push_back( ".jpg" );
-	supportedExtensions.push_back( ".png" );
-  supportedExtensions.push_back( ".tga" );
-	if ( NULL == pArchive || NULL == pArchive || NULL == pMatHelper )
-	{
+												 Q3BSP::Q3BSPZipArchive *pArchive, aiScene*,
+												 aiMaterial *pMatHelper, int textureId ) {
+	if ( NULL == pArchive || NULL == pArchive || NULL == pMatHelper ) {
 		return false;
 	}
 
-	if ( textureId < 0 || textureId >= static_cast<int>( pModel->m_Textures.size() ) )
-	{
+	if ( textureId < 0 || textureId >= static_cast<int>( pModel->m_Textures.size() ) ) {
 		return false;
 	}
 
 	bool res = true;
 	sQ3BSPTexture *pTexture = pModel->m_Textures[ textureId ];
-	if ( NULL == pTexture )
-		return false;
-
+	if ( !pTexture ) {
+        return false;
+    }
+
+    std::vector<std::string> supportedExtensions;
+    supportedExtensions.push_back( ".jpg" );
+    supportedExtensions.push_back( ".png" );
+    supportedExtensions.push_back( ".tga" );
 	std::string textureName, ext;
-	if ( expandFile( pArchive, pTexture->strName, supportedExtensions, textureName, ext ) )
-	{
+	if ( expandFile( pArchive, pTexture->strName, supportedExtensions, textureName, ext ) ) {
 		IOStream *pTextureStream = pArchive->Open( textureName.c_str() );
-		if ( NULL != pTextureStream )
-		{
+		if ( !pTextureStream ) {
 			size_t texSize = pTextureStream->FileSize();
 			aiTexture *pTexture = new aiTexture;
 			pTexture->mHeight = 0;
@@ -685,9 +673,7 @@ bool Q3BSPFileImporter::importTextureFromArchive( const Q3BSP::Q3BSPModel *pMode
 
 			pMatHelper->AddProperty( &name, AI_MATKEY_TEXTURE_DIFFUSE( 0 ) );
 			mTextures.push_back( pTexture );
-		}
-		else
-		{
+		} else {
 			// If it doesn't exist in the archive, it is probably just a reference to an external file.
 			// We'll leave it up to the user to figure out which extension the file has.
 			aiString name;

include/assimp/metadata.h

@@ -141,8 +141,8 @@ struct aiMetadata
 	/** Destructor */
 	~aiMetadata()
 	{
-		if (mKeys)
-			delete [] mKeys;
+        delete[] mKeys;
+        mKeys = NULL;
 		if (mValues)
 		{
 			// Delete each metadata entry
@@ -177,8 +177,8 @@ struct aiMetadata
 
 			// Delete the metadata array
 			delete [] mValues;
+            mValues = NULL;
 		}
-		
 	}
 
 
@@ -206,8 +206,9 @@ struct aiMetadata
 
 		// Return false if the output data type does 
 		// not match the found value's data type
-		if (GetAiType(value) != mValues[index].mType)
-			return false;
+        if ( GetAiType( value ) != mValues[ index ].mType ) {
+            return false;
+        }
 
 		// Otherwise, output the found value and 
 		// return true
@@ -226,10 +227,12 @@ struct aiMetadata
 	}
 
 	template<typename T>
-	inline bool Get( const std::string& key, T& value )
-	{ return Get(aiString(key), value); }
+	inline bool Get( const std::string& key, T& value ) {
+        return Get(aiString(key), value); 
+    }
 
 #endif // __cplusplus
+
 };
 
 #endif // __AI_METADATA_H_INC__