Free Heroes of Might and Magic 2 is an open-source project in which you want to participate

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.








All Articles