PVS-Studio es una herramienta de análisis estático que le permite encontrar errores en el código fuente de los programas. Como familiarizado con las capacidades del analizador, le ofrecemos el resultado de la verificación de PVS-Studio del código fuente del Storm Engine.
Storm Engine
Storm Engine - , 2000 . 26 2021 GPLv3 GitHub. C++.
235 High 794 Medium. , . – , .
1029 – , , . , , , . , .
PVS-Studio: V547 Expression 'nStringCode >= 0xffffff' is always false. dstring_codec.h 84
#define DHASH_SINGLESYM 255
....
uint32_t Convert(const char *pString, ....)
{
uint32_t nStringCode;
....
nStringCode = ((((unsigned char)pString[0]) << 8) & 0xffffff00) |
(DHASH_SINGLESYM)
....
if (nStringCode >= 0xffffff)
{
__debugbreak();
}
return nStringCode;
}
, nStringCode. unsigned char [0, 255], (unsigned char)pString[0] 2^8, *8 * 2^16, . 255. nStringCode 2^16+256, , 0xffffff = 2^24-1, . , , :
#define DHASH_SINGLESYM 255
....
uint32_t Convert(const char *pString, ....)
{
uint32_t nStringCode;
....
nStringCode = ((((unsigned char)pString[0]) << 8) & 0xffffff00) |
(DHASH_SINGLESYM)
....
return nStringCode;
}
, . , , , DHASH_SINGLESYM. , , , , .
PVS-Studio: V560 A part of conditional expression is always true: 0x00 <= c. utf8.h 187
inline bool IsValidUtf8(....)
{
int c, i, ix, n, j;
for (i = 0, ix = str.length(); i < ix; i++)s
{
c = (unsigned char)str[i];
if (0x00 <= c && c <= 0x7f)
n = 0;
....
}
....
}
c , 0x00 <= c , . :
inline bool IsValidUtf8(....)
{
int c, i, ix, n, j;
for (i = 0, ix = str.length(); i < ix; i++)s
{
c = (unsigned char)str[i];
if (c <= 0x7f)
n = 0;
....
}
....
}
PVS-Studio: V557 Array overrun is possible. The value of 'TempLong2 - TempLong1 + 1' index could reach 520. internal_functions.cpp 1131
DATA *COMPILER::BC_CallIntFunction(....)
{
if (TempLong2 - TempLong1 >= sizeof(Message_string))
{
SetError("internal: buffer too small");
pV = SStack.Push();
pV->Set("");
pVResult = pV;
return pV;
}
memcpy(Message_string, pChar + TempLong1,
TempLong2 - TempLong1 + 1);
Message_string[TempLong2 - TempLong1 + 1] = 0;
pV = SStack.Push();
}
off-by-one error.
, TempLong2 - TempLong1 Message_string, TempLong2 - TempLong1 + 1. , TempLong2 - TempLong1 + 1 == sizeof(Message_string), , , , . :
DATA *COMPILER::BC_CallIntFunction(....)
{
if (TempLong2 - TempLong1 + 1 >= sizeof(Message_string))
{
SetError("internal: buffer too small");
pV = SStack.Push();
pV->Set("");
pVResult = pV;
return pV;
}
memcpy(Message_string, pChar + TempLong1,
TempLong2 - TempLong1 + 1);
Message_string[TempLong2 - TempLong1 + 1] = 0;
pV = SStack.Push();
}
PVS-Studio: V570 The 'Data_num' variable is assigned to itself. s_stack.cpp 36
uint32_t Data_num;
....
DATA *S_STACK::Push(....)
{
if (Data_num > 1000)
{
Data_num = Data_num;
}
....
}
, . *Data_num * . , . , , . – 1000, . , .
PVS-Studio: V595 The 'rs' pointer was utilized before it was verified against nullptr. Check lines: 163, 164. Fader.cpp 163
uint64_t Fader::ProcessMessage(....)
{
....
textureID = rs->TextureCreate(_name);
if (rs)
{
rs->SetProgressImage(_name);
....
}
rs, nullptr. nullptr, , :
uint64_t Fader::ProcessMessage(....)
{
....
if (rs)
{
textureID = rs->TextureCreate(_name);
rs->SetProgressImage(_name);
....
}
, rs != nullptr, if (rs) , :
uint64_t Fader::ProcessMessage(....) { .... textureID = rs->TextureCreate(_name); rs->SetProgressImage(_name); .... }
. , textureID.
14 V595.
, PVS-Studio. :
PVS-Studio: V595 The 'pACh' pointer was utilized before it was verified against nullptr. Check lines: 1214, 1215. sail.cpp 1214
void SAIL::SetAllSails(int groupNum)
{
....
SetSailTextures(groupNum, core.Event("GetSailTextureData",
"l", pACh->GetAttributeAsDword("index", -1)));
if (pACh != nullptr){
....
}
*Event * pACh, nullptr. , SetSailTextures, pACh, :
void SAIL::SetAllSails(int groupNum)
{
....
if (pACh != nullptr){
SetSailTextures(groupNum, core.Event("GetSailTextureData",
"l", pACh->GetAttributeAsDword("index", -1)));
....
}
, pACh – , :
void SAIL::SetAllSails(int groupNum)
{
....
SetSailTextures(groupNum, core.Event("GetSailTextureData",
"l", pACh->GetAttributeAsDword("index", -1)));
....
}
new[] – delete
PVS-Studio: V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] pVSea;'. Check lines: 169, 191. SEA.cpp 169
struct CVECTOR
{
public:
union {
struct
{
float x, y, z;
};
float v[3];
};
};
....
struct SeaVertex
{
CVECTOR vPos;
CVECTOR vNormal;
float tu, tv;
};
....
#define STORM_DELETE (x)
{ delete x; x = 0; }
void SEA::SFLB_CreateBuffers()
{
....
pVSea = new SeaVertex[NUM_VERTEXS];
}
SEA::~SEA() {
....
STORM_DELETE(pVSea);
....
}
– , . : new[] delete *delete[]*. , *pVSea * . – , .
runtime – , . , new[] , , , . delete , , , . , . , , .
, . 15 , :
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] m_pShowPlaces;'. Check lines: 421, 196. ActivePerkShower.cpp 421
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] pTable;'. Check lines: 371, 372. AIFlowGraph.h 371
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] vrt;'. Check lines: 33, 27. OctTree.cpp 33
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] flist;'. Flag.cpp 738
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] rlist;'. Rope.cpp 660
, STORM_DELETE . delete delete[] , , new. , STORM_DELETE_ARRAY, delete[]:
struct CVECTOR
....
struct SeaVertex
{
CVECTOR vPos;
CVECTOR vNormal;
float tu, tv;
};
....
#define STORM_DELETE (x)
{ delete x; x = 0; }
#define STORM_DELETE_ARRAY (x)
{ delete[] x; x = 0; }
void SEA::SFLB_CreateBuffers()
{
....
pVSea = new SeaVertex[NUM_VERTEXS];
}
SEA::~SEA()
{
....
STORM_DELETE_ARRAY(pVSea);
....
}
PVS-Studio: V519 The 'h' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 385, 389. Sharks.cpp 389
inline void Sharks::Shark::IslandCollision(....)
{
if (h < 1.0f)
{
h -= 100.0f / 150.0f;
if (h > 0.0f)
{
h *= 150.0f / 50.0f;
}
else
h = 0.0f;
h = 0.0f;
vx -= x * (1.0f - h);
vz -= z * (1.0f - h);
}
h < 1.0f h, 0. – h 0, , h :
inline void Sharks::Shark::IslandCollision(....)
{
if (h < 1.0f)
{
h -= 100.0f / 150.0f;
if (h > 0.0f)
{
h *= 150.0f / 50.0f;
}
else
h = 0.0f;
vx -= x * (1.0f - h);
vz -= z * (1.0f - h);
}
, realloc malloc
PVS-Studio: V522 There might be dereferencing of a potential null pointer 'pTable'. Check lines: 36, 35. s_postevents.h 36
void Add(....)
{
....
pTable = (S_EVENTMSG **)realloc(
pTable, nClassesNum * sizeof(S_EVENTMSG *));
pTable[n] = pClass;
....
};
realloc , , NULL. , *pTable[n] * , . , - , pTable , . :
void Add(....)
{
....
S_EVENTMSG ** newpTable
= (S_EVENTMSG **)realloc(pTable,
nClassesNum * sizeof(S_EVENTMSG *));
if(newpTable)
{
pTable = newpTable;
pTable[n] = pClass;
....
}
else
{
// , realloc
}
};
malloc:
PVS-Studio: V522 There might be dereferencing of a potential null pointer 'label'. Check lines: 116, 113. geom_static.cpp 116
GEOM::GEOM(....) : srv(_srv)
{
....
label = static_cast<LABEL *>(srv.malloc(sizeof(LABEL) *
rhead.nlabels));
for (long lb = 0; lb < rhead.nlabels; lb++)
{
label[lb].flags = lab[lb].flags;
label[lb].name = &globname[lab[lb].name];
label[lb].group_name = &globname[lab[lb].group_name];
memcpy(&label[lb].m[0][0], &lab[lb].m[0][0],
sizeof(lab[lb].m));
memcpy(&label[lb].bones[0], &lab[lb].bones[0],
sizeof(lab[lb].bones));
memcpy(&label[lb].weight[0], &lab[lb].weight[0],
sizeof(lab[lb].weight));
}
}
:
GEOM::GEOM(....) : srv(_srv)
{
....
label = static_cast<LABEL *>(srv.malloc(sizeof(LABEL) *
rhead.nlabels));
for (long lb = 0; lb < rhead.nlabels; lb++)
{
if(label)
{
label[lb].flags = lab[lb].flags;
label[lb].name = &globname[lab[lb].name];
label[lb].group_name = &globname[lab[lb].group_name];
memcpy(&label[lb].m[0][0], &lab[lb].m[0][0],
sizeof(lab[lb].m));
memcpy(&label[lb].bones[0], &lab[lb].bones[0],
sizeof(lab[lb].bones));
memcpy(&label[lb].weight[0], &lab[lb].weight[0],
sizeof(lab[lb].weight));
}
....
}
}
18 .
1
PVS-Studio: V1063 The modulo by 1 operation is meaningless. The result will always be zero. WdmSea.cpp 205
void WdmSea::Update(float dltTime)
{
long whiteHorses[1];
....
wh[i].textureIndex = rand() % (sizeof(whiteHorses) / sizeof(long));
}
*whiteHorses, * 1, , – 0. , *whiteHorses *– . , , *rand() % (sizeof(whiteHorses) / sizeof(long)) * . , whiteHorses , . , , , .
std::vector vs std::deque
, PVS-Studio .
PVS-Studio: V826 Consider replacing the 'aLightsSort' std::vector with std::deque. Overall efficiency of operations will increase. Lights.cpp 471
void Lights::SetCharacterLights(....)
{
std::vector<long> aLightsSort;
for (i = 0; i < numLights; i++)
aLightsSort.push_back(i);
for (i = 0; i < aMovingLight.size(); i++)
{
const auto it = std::find(aLightsSort.begin(),aLightsSort.end(),
aMovingLight[i].light);
aLightsSort.insert(aLightsSort.begin(), aMovingLight[i].light);
}
}
*std::vector aLightsSort, * .
std::vector? , (reallocation) . , . ? std::vector .
std::deque. , , . std::deque , – .
*std::vector * std::deque:
void Lights::SetCharacterLights(....)
{
std::deque<long> aLightsSort;
for (i = 0; i < numLights; i++)
aLightsSort.push_back(i);
for (i = 0; i < aMovingLight.size(); i++)
{
const auto it = std::find(aLightsSort.begin(),aLightsSort.end(),
aMovingLight[i].light);
aLightsSort.push_front(aMovingLight[i].light);
}
}
PVS-Studio Storm Engine. , – , , , (code review). ( ) – , , – , . Storm Engine , . – .
, : Yo, Ho, Ho, And a Bottle of Rum - Or How We Analyzed Storm Engine's Bugs.