Merge pull request #202 from toxieainc/analysis_fixes

Static Analysis

Src/BlockFile.h

@@ -132,7 +132,7 @@ class CBlockFile
    *    name      Block name. Must be unique and not NULL.
    *    comment   Comment string to embed in the block header.
    */
-  void NewBlock(const std::string &title, const std::string &comment);
+  void NewBlock(const std::string &name, const std::string &comment);
 
   /*
    * Create(file, headerName, comment):

Src/CPU/PowerPC/PPCDisasm.cpp

@@ -655,7 +655,7 @@ static bool Simplified(uint32_t op, uint32_t vpc, char *signed16, char *mnem, ch
             sprintf(oprs, "r%d,r%d", G_RA(op), G_RT(op));
         }
         else
-            return 0;
+            return false;
     }
     else if ((op & ~(M_RT|M_RA|M_RB|M_RC)) == (D_OP(31)|D_XO(124)))
     {
@@ -666,7 +666,7 @@ static bool Simplified(uint32_t op, uint32_t vpc, char *signed16, char *mnem, ch
             sprintf(oprs, "r%d,r%d", G_RA(op), G_RT(op));
         }
         else
-            return 0;
+            return false;
     }
     else if ((op & ~(M_RT|M_RA|M_SIMM)) == D_OP(14))
     {
@@ -676,7 +676,7 @@ static bool Simplified(uint32_t op, uint32_t vpc, char *signed16, char *mnem, ch
             sprintf(oprs, "r%d,0x%08X", G_RT(op), value);
         }
         else
-            return 0;
+            return false;
     }
     else if ((op & ~(M_RT|M_RA|M_SIMM)) == D_OP(15))
     {
@@ -757,7 +757,7 @@ static bool Simplified(uint32_t op, uint32_t vpc, char *signed16, char *mnem, ch
             strcat(mnem, "bt");
             break;
         default:
-            return 0;
+            return false;
         }
 
         if (op & M_LK)  strcat(mnem, "l");
@@ -780,8 +780,8 @@ static bool Simplified(uint32_t op, uint32_t vpc, char *signed16, char *mnem, ch
     sprintf(oprs, "r%d,r%d,r%d", G_RT(op), G_RB(op), G_RA(op));
     }
     else
-        return 0;   // no match
-  return 1;
+        return false;   // no match
+  return true;
 }
 
 /*
@@ -821,7 +821,7 @@ Result DisassemblePowerPC(uint32_t op, uint32_t vpc, char *mnem, char *oprs,
      * Decode signed 16-bit fields (SIMM and d) to spare us the work later
      */
 
-    DecodeSigned16(signed16, op, 0);
+    DecodeSigned16(signed16, op, false);
 
     /*
      * Try simplified forms first, then real instructions

Src/CPU/PowerPC/PPCDisasm.h

@@ -28,6 +28,8 @@
 #ifndef INCLUDED_PPCDISASM_H
 #define INCLUDED_PPCDISASM_H
 
+#include "Types.h"
+
 /*
  * DisassemblePowerPC(op, vpc, mnem, oprs, simplify):
  *

Src/CPU/PowerPC/ppc.cpp

@@ -117,12 +117,12 @@ static UINT32		ppc_field_xlat[256];
 
 
 
-#define BITMASK_0(n)	(UINT32)(((UINT64)1 << n) - 1)
-#define CRBIT(x)		((ppc.cr[x / 4] & (1 << (3 - (x % 4)))) ? 1 : 0)
+#define BITMASK_0(n)	(UINT32)(((UINT64)1 << (n)) - 1)
+#define CRBIT(x)		((ppc.cr[(x) / 4] & (1 << (3 - ((x) % 4)))) ? 1 : 0)
 #define _BIT(n)			(1 << (n))
 #define GET_ROTATE_MASK(mb,me)		(ppc_rotate_mask[mb][me])
-#define ADD_CA(r,a,b)		((UINT32)r < (UINT32)a)
-#define SUB_CA(r,a,b)		(!((UINT32)a < (UINT32)b))
+#define ADD_CA(r,a,b)		((UINT32)(r) < (UINT32)(a))
+#define SUB_CA(r,a,b)		(!((UINT32)(a) < (UINT32)(b)))
 #define ADD_OV(r,a,b)		((~((a) ^ (b)) & ((a) ^ (r))) & 0x80000000)
 #define SUB_OV(r,a,b)		(( ((a) ^ (b)) & ((a) ^ (r))) & 0x80000000)
 
@@ -154,8 +154,8 @@ static UINT32		ppc_field_xlat[256];
 #define TSR_ENW			0x80000000
 #define TSR_WIS			0x40000000
 
-#define BYTE_REVERSE16(x)	(((x >> 8) | (x << 8)) & 0xFFFF)
-#define BYTE_REVERSE32(x)	((x >> 24) | ((x << 8) & 0x00FF0000) | ((x >> 8) & 0x0000FF00) | (x << 24))
+#define BYTE_REVERSE16(x)	((((x) >> 8) | ((x) << 8)) & 0xFFFF)
+#define BYTE_REVERSE32(x)	(((x) >> 24) | (((x) << 8) & 0x00FF0000) | (((x) >> 8) & 0x0000FF00) | ((x) << 24))
 
 typedef union {
 	UINT64	id;
@@ -714,10 +714,9 @@ void ppc_base_init(void)
 	/* Calculate rotate mask table */
 	for( i=0; i < 32; i++ ) {
 		for( j=0; j < 32; j++ ) {
-			UINT32 mask;
 			int mb = i;
 			int me = j;
-			mask = ((UINT32)0xFFFFFFFF >> mb) ^ ((me >= 31) ? 0 : ((UINT32)0xFFFFFFFF >> (me + 1)));
+			UINT32 mask = ((UINT32)0xFFFFFFFF >> mb) ^ ((me >= 31) ? 0 : ((UINT32)0xFFFFFFFF >> (me + 1)));
 			if( mb > me )
 				mask = ~mask;
 

Src/GameLoader.cpp

@@ -363,8 +363,8 @@ bool GameLoader::LoadGamesFromXML(const Util::Config::Node &xml)
 
           // Look up region structure or create new one if needed
           std::string region_name = region_node["name"].Value<std::string>();
-          auto it = regions_by_name.find(region_name);
-          Region::ptr_t region = (it != regions_by_name.end()) ? it->second : Region::Create(*this, region_node);
+          auto it2 = regions_by_name.find(region_name);
+          Region::ptr_t region = (it2 != regions_by_name.end()) ? it2->second : Region::Create(*this, region_node);
           if (!region)
             continue;
 
@@ -432,7 +432,7 @@ bool GameLoader::LoadGamesFromXML(const Util::Config::Node &xml)
 
 static bool IsChildSet(const Game &game)
 {
-  return game.parent.length() > 0;
+  return !game.parent.empty();
 }
 
 bool GameLoader::MergeChildrenWithParents()
@@ -587,7 +587,7 @@ void GameLoader::IdentifyGamesInZipArchive(
       Region::ptr_t region = v2.second;
       if (!region->required)
         continue;
-      for (auto file: region->files)
+      for (const auto &file: region->files)
       {
         // Add each file to the set of required files per game
         files_required_by_game[game_name].insert(file);
@@ -757,7 +757,7 @@ bool GameLoader::ComputeRegionSize(uint32_t *region_size, const GameLoader::Regi
   // use maximum end_addr = offset + stride * (num_chunks - 1) + chunk_size.
   std::vector<uint32_t> end_addr;
   bool error = false;
-  for (auto file: region->files)
+  for (const auto &file: region->files)
   {
     const ZippedFile *zipped_file = LookupFile(file, zip);
     if (zipped_file)
@@ -767,7 +767,7 @@ bool GameLoader::ComputeRegionSize(uint32_t *region_size, const GameLoader::Regi
         ErrorLog("File '%s' in '%s' is not sized in %d-byte chunks.", zipped_file->filename.c_str(), zipped_file->zipfilename.c_str(), region->chunk_size);
         error = true;
       }
-	    uint32_t num_chunks = (uint32_t)(zipped_file->uncompressed_size / region->chunk_size);
+      uint32_t num_chunks = (uint32_t)(zipped_file->uncompressed_size / region->chunk_size);
       end_addr.push_back(file->offset + region->stride * (num_chunks - 1) + region->chunk_size);
     }
     else
@@ -781,7 +781,7 @@ bool GameLoader::ComputeRegionSize(uint32_t *region_size, const GameLoader::Regi
 static bool ApplyLayout(ROM *rom, const std::string &byte_layout, size_t stride, const std::string &region_name)
 {
   // Empty layout means do nothing
-  if (byte_layout.size() == 0)
+  if (byte_layout.empty())
     return false;
 
   // Validate that the layout string includes the same number of bytes as the region stride. The

Src/GameLoader.h

@@ -99,8 +99,6 @@ class GameLoader
   void ChooseGameInZipArchive(std::string *chosen_game, bool *missing_parent_roms, const ZipArchive &zip, const std::string &zipfilename) const;
   bool LoadRegion(ROM *buffer, const GameLoader::Region::ptr_t &region, const ZipArchive &zip) const;
   bool LoadROMs(ROMSet *rom_set, const std::string &game_name, const ZipArchive &zip) const;
-  std::string ChooseGame(const std::set<std::string> &games_found, const std::string &zipfilename) const;
-  static bool CompareFilesByName(const File::ptr_t &a,const File::ptr_t &b);
 
 public:
   GameLoader(const std::string &xml_file);

Src/Graphics/Legacy3D/Legacy3D.cpp

@@ -441,7 +441,7 @@ const UINT32 *CLegacy3D::TranslateModelAddress(UINT32 modelAddr)
 ******************************************************************************/
 
 // Macro to generate column-major (OpenGL) index from y,x subscripts
-#define CMINDEX(y,x)  (x*4+y)
+#define CMINDEX(y,x)  ((x)*4+(y))
 
 /*
  * MultMatrix():
@@ -527,7 +527,7 @@ void CLegacy3D::InitMatrixStack(UINT32 matrixBaseAddr)
   }
 
   // Set matrix base address and apply matrix #0 (coordinate system matrix)
-  matrixBasePtr = (float *) TranslateCullingAddress(matrixBaseAddr);
+  matrixBasePtr = (const float *) TranslateCullingAddress(matrixBaseAddr);
   MultMatrix(0);
 }
 
@@ -547,7 +547,7 @@ static bool IsDynamicModel(const UINT32 *data)
 {
   if (data == NULL)
     return false;
-  unsigned sharedVerts[16] = { 0,1,1,2,1,2,2,3,1,2,2,3,2,3,3,4 };
+  static constexpr unsigned sharedVerts[16] = { 0,1,1,2,1,2,2,3,1,2,2,3,2,3,3,4 };
   // VROM models are only dynamic if they reference polygon RAM via color palette indices
   bool done = false;
   do
@@ -561,7 +561,7 @@ static bool IsDynamicModel(const UINT32 *data)
     unsigned numVerts = (data[0]&0x40 ? 4 : 3);
     // Deduct number of reused verts
     numVerts -= sharedVerts[data[0]&0xf];
-	done = (data[1] & 4) > 0;
+    done = (data[1] & 4) > 0;
     // Skip header and vertices to next polygon
     data += 7 + numVerts * 4;
   }
@@ -1353,22 +1353,21 @@ CLegacy3D::~CLegacy3D(void)
   if (glBindBuffer != NULL) // we may have failed earlier due to lack of OpenGL 2.0 functions 
     glBindBuffer(GL_ARRAY_BUFFER, 0); // disable VBOs by binding to 0
   glDeleteTextures(numTexMaps, texMapIDs);
-  
+
   DestroyModelCache(&VROMCache);
   DestroyModelCache(&PolyCache);
-  
+
   cullingRAMLo = NULL;
   cullingRAMHi = NULL;
   polyRAM = NULL;
   vrom = NULL;
   textureRAM = NULL;
-
-  if (texSheets != NULL)
-    delete [] texSheets;
 
-  if (textureBuffer != NULL)
-    delete [] textureBuffer;
-  textureBuffer = NULL;
+  delete [] texSheets;
+  texSheets = nullptr;
+
+  delete [] textureBuffer;
+  textureBuffer = nullptr;
 
   DebugLog("Destroyed Legacy3D\n");
 }

Src/Graphics/Legacy3D/Models.cpp

@@ -160,7 +160,7 @@ namespace Legacy3D {
 ******************************************************************************/
 
 // Macro to generate column-major (OpenGL) index from y,x subscripts
-#define CMINDEX(y,x)  (x*4+y)
+#define CMINDEX(y,x)  ((x)*4+(y))
 
 static void CrossProd(GLfloat out[3], const GLfloat a[3], const GLfloat b[3])
 {
@@ -190,12 +190,11 @@ static GLfloat Sign(GLfloat x)
 // 4x4 matrix with the extra components undefined (do not use them!)
 static void InvertTransposeMat3(GLfloat out[4*4], GLfloat m[4*4])
 {
-  GLfloat invDet;
   GLfloat a00 = m[CMINDEX(0,0)], a01 = m[CMINDEX(0,1)], a02 = m[CMINDEX(0,2)];
   GLfloat a10 = m[CMINDEX(1,0)], a11 = m[CMINDEX(1,1)], a12 = m[CMINDEX(1,2)];
   GLfloat a20 = m[CMINDEX(2,0)], a21 = m[CMINDEX(2,1)], a22 = m[CMINDEX(2,2)];
 
-  invDet = 1.0f/(a00*(a22*a11-a21*a12)-a10*(a22*a01-a21*a02)+a20*(a12*a01-a11*a02));
+  GLfloat invDet = 1.0f/(a00*(a22*a11-a21*a12)-a10*(a22*a01-a21*a02)+a20*(a12*a01-a11*a02));
   out[CMINDEX(0,0)] = invDet*(a22*a11-a21*a12);   out[CMINDEX(1,0)] = invDet*(-(a22*a01-a21*a02));  out[CMINDEX(2,0)] = invDet*(a12*a01-a11*a02);
   out[CMINDEX(0,1)] = invDet*(-(a22*a10-a20*a12));  out[CMINDEX(1,1)] = invDet*(a22*a00-a20*a02);   out[CMINDEX(2,1)] = invDet*(-(a12*a00-a10*a02));
   out[CMINDEX(0,2)] = invDet*(a21*a10-a20*a11);   out[CMINDEX(1,2)] = invDet*(-(a21*a00-a20*a01));  out[CMINDEX(2,2)] = invDet*(a11*a00-a10*a01);
@@ -390,8 +389,8 @@ Result CLegacy3D::AppendDisplayList(ModelCache *Cache, bool isViewport, const st
        * This is described further in InsertPolygon(), where the vertices
        * are ordered in clockwise fashion.
        */
-      static const GLfloat x[3] = { 1.0f, 0.0f, 0.0f };
-      static const GLfloat y[3] = { 0.0f, 1.0f, 0.0f };
+      static constexpr GLfloat x[3] = { 1.0f, 0.0f, 0.0f };
+      static constexpr GLfloat y[3] = { 0.0f, 1.0f, 0.0f };
       const GLfloat z[3] = { 0.0f, 0.0f, -1.0f*matrixBasePtr[0x5] };
       GLfloat m[4*4];
       GLfloat xT[3], yT[3], zT[3], pT[3];
@@ -1268,7 +1267,7 @@ Result CLegacy3D::CreateModelCache(ModelCache *Cache, unsigned vboMaxVerts,
   // Clear LUT (MUST be done here because ClearModelCache() won't do it for dynamic models)
   for (size_t i = 0; i < numLUTEntries; i++)
     Cache->lut[i] = -1;
-    
+
   // All good!
   return Result::OKAY;
 }
@@ -1279,16 +1278,12 @@ void CLegacy3D::DestroyModelCache(ModelCache *Cache)
 
   for (size_t i = 0; i < 2; i++)
   {
-    if (Cache->verts[i] != NULL)
-      delete [] Cache->verts[i];
+    delete [] Cache->verts[i];
   }
-  if (Cache->Models != NULL)
-    delete [] Cache->Models;
-  if (Cache->lut != NULL)
-    delete [] Cache->lut;
-  if (Cache->List != NULL)
-    delete [] Cache->List;
-
+  delete [] Cache->Models;
+  delete [] Cache->lut;
+  delete [] Cache->List;
+
   memset(Cache, 0, sizeof(ModelCache));
 }
 

Src/Graphics/Legacy3D/Shaders3D.h

@@ -31,7 +31,7 @@
 namespace Legacy3D {
 
 // Vertex shader
-static const char vertexShaderSource[] =
+static constexpr char vertexShaderSource[] =
 {
 "/**\n"
 " ** Supermodel\n"
@@ -246,7 +246,7 @@ static const char vertexShaderSource[] =
 
 
 // Fragment shader (single texture sheet)
-static const char fragmentShaderSingleSheetSource[] = 
+static constexpr char fragmentShaderSingleSheetSource[] = 
 {
 "/**\n"
 " ** Supermodel\n"
@@ -446,7 +446,7 @@ static const char fragmentShaderSingleSheetSource[] =
 
 
 // Fragment shader (8 texture sheets)
-static const char fragmentShaderMultiSheetSource[] = 
+static constexpr char fragmentShaderMultiSheetSource[] = 
 {
 "/**\n"
 " ** Supermodel\n"

Src/Graphics/Legacy3D/TextureRefs.cpp

@@ -39,7 +39,7 @@ namespace Legacy3D {
 
 CTextureRefs::CTextureRefs() : m_size(0), m_hashCapacity(0), m_hashEntries(NULL)
 {
-	//
+	memset(m_array, 0, sizeof(m_array));
 }
 
 CTextureRefs::~CTextureRefs()
@@ -261,8 +261,7 @@ void CTextureRefs::DeleteAllHashEntries()
 	for (unsigned i = 0; i < m_hashCapacity; i++)
 	{
 		HashEntry *entry = m_hashEntries[i];
-		if (entry)
-			delete entry;
+		delete entry;
 	}
 	delete[] m_hashEntries;
 }

Src/Graphics/New3D/Model.h

@@ -2,7 +2,6 @@
 #define _MODEL_H_
 
 #include <vector>
-#include <unordered_map>
 #include <memory>
 #include <cstring>
 #include "Types.h"

Src/Graphics/New3D/New3D.h

@@ -28,6 +28,7 @@
 #ifndef INCLUDED_NEW3D_H
 #define INCLUDED_NEW3D_H
 
+#include <unordered_map>
 #include <GL/glew.h>
 #include "Types.h"
 #include "Graphics/IRender3D.h"

Src/Graphics/New3D/R3DFrameBuffers.h

@@ -2,7 +2,6 @@
 #define FBO_H
 
 #include <GL/glew.h>
-#include "VBO.h"
 #include "GLSLShader.h"
 #include "Model.h"
 

Src/Graphics/Render2D.h

@@ -148,7 +148,7 @@ class CRender2D
 	*
 	* Parameters:
 	*    bottom		Bottom surface
-	*	 top		Top surface	
+	*    top		Top surface	
 	*/
 	void AttachDrawBuffers(std::shared_ptr<TileGenBuffer> bottom, std::shared_ptr<TileGenBuffer> top);