Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static Analysis #202

Merged
merged 3 commits into from
Oct 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Src/BlockFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
14 changes: 7 additions & 7 deletions Src/CPU/PowerPC/PPCDisasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
{
Expand All @@ -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))
{
Expand All @@ -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))
{
Expand Down Expand Up @@ -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");
Expand All @@ -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;
}

/*
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions Src/CPU/PowerPC/PPCDisasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#ifndef INCLUDED_PPCDISASM_H
#define INCLUDED_PPCDISASM_H

#include "Types.h"

/*
* DisassemblePowerPC(op, vpc, mnem, oprs, simplify):
*
Expand Down
15 changes: 7 additions & 8 deletions Src/CPU/PowerPC/ppc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
14 changes: 7 additions & 7 deletions Src/GameLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions Src/GameLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
23 changes: 11 additions & 12 deletions Src/Graphics/Legacy3D/Legacy3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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);
}

Expand All @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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");
}
Expand Down
25 changes: 10 additions & 15 deletions Src/Graphics/Legacy3D/Models.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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])
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
}
Expand All @@ -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));
}

Expand Down
6 changes: 3 additions & 3 deletions Src/Graphics/Legacy3D/Shaders3D.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
namespace Legacy3D {

// Vertex shader
static const char vertexShaderSource[] =
static constexpr char vertexShaderSource[] =
{
"/**\n"
" ** Supermodel\n"
Expand Down Expand Up @@ -246,7 +246,7 @@ static const char vertexShaderSource[] =


// Fragment shader (single texture sheet)
static const char fragmentShaderSingleSheetSource[] =
static constexpr char fragmentShaderSingleSheetSource[] =
{
"/**\n"
" ** Supermodel\n"
Expand Down Expand Up @@ -446,7 +446,7 @@ static const char fragmentShaderSingleSheetSource[] =


// Fragment shader (8 texture sheets)
static const char fragmentShaderMultiSheetSource[] =
static constexpr char fragmentShaderMultiSheetSource[] =
{
"/**\n"
" ** Supermodel\n"
Expand Down
5 changes: 2 additions & 3 deletions Src/Graphics/Legacy3D/TextureRefs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -261,8 +261,7 @@ void CTextureRefs::DeleteAllHashEntries()
for (unsigned i = 0; i < m_hashCapacity; i++)
{
HashEntry *entry = m_hashEntries[i];
if (entry)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this check isn't needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, nowadays all compilers respect the c++ standard, that says 'In all cases, if ptr is a null pointer, the standard library deallocation functions do nothing.'

delete entry;
delete entry;
}
delete[] m_hashEntries;
}
Expand Down
1 change: 0 additions & 1 deletion Src/Graphics/New3D/Model.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define _MODEL_H_

#include <vector>
#include <unordered_map>
#include <memory>
#include <cstring>
#include "Types.h"
Expand Down
1 change: 1 addition & 0 deletions Src/Graphics/New3D/New3D.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion Src/Graphics/New3D/R3DFrameBuffers.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define FBO_H

#include <GL/glew.h>
#include "VBO.h"
#include "GLSLShader.h"
#include "Model.h"

Expand Down
2 changes: 1 addition & 1 deletion Src/Graphics/Render2D.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading