widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Approve

3 nits about initializing std::string = "", otherwise lgtm. I think this can go in once you have a look at the nits.

Diff comments:

> 
> === modified file 'src/game_io/game_preload_packet.h'
> --- src/game_io/game_preload_packet.h	2017-01-25 18:55:59 +0000
> +++ src/game_io/game_preload_packet.h	2017-08-10 07:51:51 +0000
> @@ -74,16 +74,17 @@
>  	}
>  
>  private:
> -	std::string minimap_path_;
> -	std::string mapname_;
> -	std::string background_;
> -	std::string win_condition_;
> -	uint32_t gametime_;
> -	uint8_t player_nr_;  // The local player idx
> -	uint8_t number_of_players_;
> -	std::string version_;
> -	time_t savetimestamp_;
> -	GameController::GameType gametype_;
> +	// Initializing everything to make cppcheck happy.
> +	std::string minimap_path_ = "";

Initializing a std::string is unnecessary, it is always the empty string. Does cppcheck really complain about this?

> +	std::string mapname_ = "";
> +	std::string background_ = "";
> +	std::string win_condition_ = "";
> +	uint32_t gametime_ = 0U;
> +	uint8_t player_nr_ = 0U;  // The local player idx
> +	uint8_t number_of_players_ = 0U;
> +	std::string version_ = "";
> +	time_t savetimestamp_ = 0;
> +	GameController::GameType gametype_ = GameController::GameType::kUndefined;
>  };
>  }
>  
> 
> === modified file 'src/map_io/map_elemental_packet.cc'
> --- src/map_io/map_elemental_packet.cc	2017-01-25 18:55:59 +0000
> +++ src/map_io/map_elemental_packet.cc	2017-08-10 07:51:51 +0000
> @@ -33,6 +33,9 @@
>  constexpr int32_t kEightPlayersPacketVersion = 1;
>  constexpr int32_t kSixteenPlayersPacketVersion = 2;
>  
> +MapElementalPacket::MapElementalPacket() : old_world_name_(""), version_(0) {

no need for std::string initialization?

> +}
> +
>  void MapElementalPacket::pre_read(FileSystem& fs, Map* map) {
>  	Profile prof;
>  	prof.read("elemental", nullptr, fs);
> 
> === modified file 'src/network/gameclient.cc'
> --- src/network/gameclient.cc	2017-07-05 19:21:57 +0000
> +++ src/network/gameclient.cc	2017-08-10 07:51:51 +0000
> @@ -820,8 +820,7 @@
>  	}
>  
>  	case NETCMD_CHAT: {
> -		ChatMessage c;
> -		c.time = time(nullptr);
> +		ChatMessage c("");

Here too?

>  		c.playern = packet.signed_16();
>  		c.sender = packet.string();
>  		c.msg = packet.string();


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables/+merge/326256
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-986611-cppcheck-uninitialized-variables.