Recently, news about the release of a new version of the fheroes2 project appeared on the network. In our company, many employees are fans of the Heroes of Might and Magic series of games, and naturally, we could not pass by, and in the process of getting to know the project we checked it with the PVS-Studio analyzer.
Getting to know the project
Free Heroes of Might and Magic II – Heroes of Might and Magic II . , Heroes of Might and Magic II , ( ).
. fheroes2.cfg, :
heroes speed = 10
ai speed = 10
battle speed = 10
videomode.
:
, f4.
, standard game. , – Broken Alliance.
, , . , , , , . , .
0.8.4, , , . : " ". , , , GitHub, Sonar Cxx , Cppcheck.
, , PVS-Studio, . , . PVS-Studio .
, , . , , . , , , . , , "" PVS-Studio, .
N1
V823 Decreased performance. Object may be created in-place in the 'list' container. Consider replacing methods: 'push_back' -> 'emplace_back'. tools.cpp 231
std::list StringSplit( const std::string & str, ....)
{
....
while (....)
{
list.push_back( str.substr( pos1, pos2 - pos1 ) );
....
}
....
}
, emplace_back. , , .
, , . :
V823 Decreased performance. Object may be created in-place in the 'loop_sounds' container. Consider replacing methods: 'push_back' -> 'emplace_back'. agg.cpp 461
V823 Decreased performance. Object may be created in-place in the 'projectileOffset' container. Consider replacing methods: 'push_back' -> 'emplace_back'. bin_info.cpp 183
V823 Decreased performance. Object may be created in-place in the 'actions' container. Consider replacing methods: 'push_back' -> 'emplace_back'. ai_normal_battle.cpp 264
V823 Decreased performance. Object may be created in-place in the 'actions' container. Consider replacing methods: 'push_back' -> 'emplace_back'. ai_normal_battle.cpp 288
V823 Decreased performance. Object may be created in-place in the 'actions' container. Consider replacing methods: 'push_back' -> 'emplace_back'. ai_normal_battle.cpp 433
N2
V814 Decreased performance. The 'strlen' function was called multiple times inside the body of a loop. tools.cpp 216
void StringReplace( std::string & dst,
const char * pred,
const std::string & src )
{
size_t pos = std::string::npos;
while ( std::string::npos != ( pos = dst.find( pred ) ) )
{
dst.replace( pos, std::strlen( pred ), src );
}
}
strlen , pred . , .
void StringReplace( std::string & dst,
const char * pred,
const std::string & src )
{
size_t pos = std::string::npos;
const size_t predSize = std::strlen( pred);
while ( std::string::npos != ( pos = dst.find( pred ) ) )
{
dst.replace( pos, predSize, src );
}
}
N3
V827 Maximum size of the 'optionAreas' vector is known at compile time. Consider pre-allocating it by calling optionAreas.reserve(6) battle_dialogs.cpp 217
void Battle::DialogBattleSettings( .... )
{
std::vector optionAreas;
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 36,
pos_rt.y + 47,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 128,
pos_rt.y + 47,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 220,
pos_rt.y + 47,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 36,
pos_rt.y + 157,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 128,
pos_rt.y + 157,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 220,
pos_rt.y + 157,
panelWidth,
panelHeight ) );
}
std::vector, . :
optionAreas.reserve(6);
push_back . , , std::array.
N4.0, 4.1...4.7
V809 Verifying that a pointer value is not NULL is not required. The 'if (armyBar)' check can be removed. kingdom_overview.cpp 62
V809 Verifying that a pointer value is not NULL is not required. The 'if (artifactsBar)' check can be removed. kingdom_overview.cpp 64
V809 Verifying that a pointer value is not NULL is not required. The 'if (secskillsBar)' check can be removed. kingdom_overview.cpp 66
V809 Verifying that a pointer value is not NULL is not required. The 'if (primskillsBar)' check can be removed. kingdom_overview.cpp 68
V809 Verifying that a pointer value is not NULL is not required. The 'if (armyBarGuard)' check can be removed. kingdom_overview.cpp 279
V809 Verifying that a pointer value is not NULL is not required. The 'if (armyBarGuest)' check can be removed. kingdom_overview.cpp 281
V809 Verifying that a pointer value is not NULL is not required. The 'if (dwellingsBar)' check can be removed. kingdom_overview.cpp 283
Clear, , .
void Clear( void )
{
if ( armyBar )
delete armyBar;
if ( artifactsBar )
delete artifactsBar;
if ( secskillsBar )
delete secskillsBar;
if ( primskillsBar )
delete primskillsBar;
}
void Clear( void )
{
if ( armyBarGuard )
delete armyBarGuard;
if ( armyBarGuest )
delete armyBarGuest;
if ( dwellingsBar )
delete dwellingsBar;
}
, , delete . , ( ), .
General Analysis
N5
2 :
V654 The condition 'i < originalPalette.size()' of loop is always false. battle_interface.cpp 3689
V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. battle_interface.cpp 3689
void Battle::Interface::RedrawActionBloodLustSpell( Unit & target )
{
std::vector > originalPalette;
if ( target.Modes( SP_STONE ) )
{
originalPalette.push_back( PAL::GetPalette( PAL::GRAY ) );
}
else if ( target.Modes( CAP_MIRRORIMAGE ) )
{
originalPalette.push_back( PAL::GetPalette( PAL::MIRROR_IMAGE ) );
}
if ( !originalPalette.empty() )
{
for ( size_t i = 1; i < originalPalette.size(); ++i )
{
originalPalette[0] = PAL::CombinePalettes( originalPalette[0],
originalPalette[i] );
}
fheroes2::ApplyPalette( unitSprite, originalPalette[0] );
}
....
}
, . originalPalette . if , , originalPalette.size() . , i , - .
N6
V547 Expression 'palette.empty()' is always true. image_tool.cpp 32
const std::vector PALPAlette()
{
std::vector palette;
if (palette.empty()) //<=
{
palette.resize( 256 * 3 );
for ( size_t i = 0; i < palette.size(); ++i )
{
palette[i] = kb_pal[i] << 2;
}
}
return palette;
}
, , , , , , .
N7
V668 There is no sense in testing the 'listlog' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. battle_interface.cpp 986
Battle::Interface::Interface(....)
{
....
listlog = new StatusListBox();
....
if ( listlog )
{
....
}
....
}
, , new, . , , , . new , , ++, std::bad_alloc() , .
:
V668 There is no sense in testing the 'elem' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. battle_arena.cpp 1079
V668 There is no sense in testing the 'image' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. battle_arena.cpp 1095
N8
V595 The '_currentUnit' pointer was utilized before it was verified against nullptr. Check lines: 2336, 2358. battle_interface.cpp 2336
void Battle::Interface::MouseLeftClickBoardAction( .... )
{
....
themes = GetSwordCursorDirection( Board::GetDirection( index,
_currentUnit->GetHeadIndex()));
....
if ( _currentUnit )
{
....
}
....
}
_currentUnit , NULL. : , , , . , . .
, . , , . , , PVS-Studio, , , open-source .
, . open-source , , fheroes2 – , .
, : Vladislav Stolyarov. Free Heroes of Might and Magic II: Open-Source Project that You Want to Be Part of.