Member variable DefaultAI::enemysites_check_delay_ used before initialization

[Jukka Pakarinen]

After playing ten minutes the Crater map in Single Player mode against AI, Messages Inbox shows up with a Status message. In that moment Valgrind detects there is something that isn't initialized before it is used.

==18778== Conditional jump or move depends on uninitialised value(s)
==18778== at 0xF36229: DefaultAI::check_enemy_sites(unsigned int) (defaultai_warfare.cc:459)
==18778== by 0xED5197: DefaultAI::think() (defaultai.cc:438)
==18778== by 0xC97D1D: SinglePlayerGameController::think() (single_player_game_controller.cc:72)
==18778== by 0xB535D4: Widelands::Game::think() (game.cc:545)
==18778== by 0xDA0192: InteractiveBase::think() (interactive_base.cc:388)
==18778== by 0xDBB2EF: InteractivePlayer::think() (interactive_player.cc:270)
==18778== by 0xD112D6: UI::Panel::do_think() (panel.cc:456)
==18778== by 0xD10966: UI::Panel::do_run() (panel.cc:181)
==18778== by 0xA31433: UI::Panel::Returncodes UI::Panel::run<UI::Panel::Returncodes>() (panel.h:99)
==18778== by 0xB53292: Widelands::Game::run(UI::ProgressWindow*, Widelands::Game::StartGameType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (game.cc:522)
==18778== by 0xA29FC6: WLApplication::new_game() (wlapplication.cc:1254)
==18778== by 0xA293D3: WLApplication::mainmenu_singleplayer() (wlapplication.cc:1112)
==18778==

I tracked the problem and found that enemysites_check_delay_ member varriable of DefaulAI structure is not initialized. It might be a good idea to initialize it to some value. I don't know which value would be suitable.

[Jukka Pakarinen (flegu)]

I tried to fix this and did the following change which removed the Valgrind message.

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc 2017-11-20 07:54:19 +0000
+++ src/ai/defaultai.cc 2017-11-26 15:21:25 +0000
@@ -106,6 +106,7 @@
      ts_in_const_count_(0),
      ts_without_trainers_(0),
      inhibit_road_building_(0),
+ enemysites_check_delay_(30),
      resource_necessity_water_needed_(false),
      highest_nonmil_prio_(0),
      seafaring_economy(false),

[GunChleoc (gunchleoc)]

A natural fix would be to set it to 0, which is what one would expect of a new integer. Do you want to make a branch?

[TiborB (tiborb95)]

30 is OK, I just wonder how it could work without this...

[GunChleoc (gunchleoc)]

The behavior is undefined. Maybe we just got lucky that it's mostly something useful.

Compilers don't verify that variables were properly initialized.

[Jukka Pakarinen (flegu)]

I initialized it to zero and made merge 334287 request.

[GunChleoc (gunchleoc)]

Fixed in build20-rc1