PVS-Studio is a static analysis tool that allows you to find errors in the source code of programs. As an acquaintance with the analyzer's capabilities, we offer you the result of PVS-Studio's check of the source code of the 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.