From 851ef445ab168a5e50ce0bac54a8bec9409faac4 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 19:55:37 +0200 Subject: [PATCH 01/14] Fix comment typos. --- src/gui/dialogs/campaign_selection.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/dialogs/campaign_selection.hpp b/src/gui/dialogs/campaign_selection.hpp index cd89a4a485a..5eb5a3c2a9a 100644 --- a/src/gui/dialogs/campaign_selection.hpp +++ b/src/gui/dialogs/campaign_selection.hpp @@ -62,7 +62,7 @@ private: /** The chosen campaign. */ int choice_; - /** whether zhe player checked teh "Deterministic" checkbox. */ + /** whether the player checked the "Deterministic" checkbox. */ bool deterministic_; }; From ef1e0e199b714846980ecae5bce3cff317106dea Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 19:58:21 +0200 Subject: [PATCH 02/14] Initialise all members. Issue found by cppcheck. --- src/gui/dialogs/campaign_selection.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/dialogs/campaign_selection.hpp b/src/gui/dialogs/campaign_selection.hpp index 5eb5a3c2a9a..fd5c91f0082 100644 --- a/src/gui/dialogs/campaign_selection.hpp +++ b/src/gui/dialogs/campaign_selection.hpp @@ -26,7 +26,7 @@ class tcampaign_selection : public tdialog { public: explicit tcampaign_selection(const std::vector& campaigns) - : campaigns_(campaigns), choice_(-1) + : campaigns_(campaigns), choice_(-1), deterministic_(false) { } From 5acb30af5fe6960370ce12cf0eb89f93e120450a Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:08:00 +0200 Subject: [PATCH 03/14] Pre instead of post increment a variable. Issue found by cppcheck. --- src/game_preferences_display.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/game_preferences_display.cpp b/src/game_preferences_display.cpp index 3a354727358..30385c5ad1c 100644 --- a/src/game_preferences_display.cpp +++ b/src/game_preferences_display.cpp @@ -308,7 +308,7 @@ preferences_dialog::preferences_dialog(display& disp, const config& game_cfg) const std::map& colors = game_config::team_rgb_name; std::map::const_iterator colors_it; - for (colors_it = colors.begin(); colors_it != colors.end(); colors_it++) { + for (colors_it = colors.begin(); colors_it != colors.end(); ++colors_it) { const std::string& color_id = colors_it->first; const t_string& color_name = colors_it->second; From 2f780b96bebb59fe8b0ce26754e28954495b52e7 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:16:40 +0200 Subject: [PATCH 04/14] Replace a increment loop with std::advance. The code was flagged by cppcheck, due to the post increment. --- src/playturn_network_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playturn_network_adapter.cpp b/src/playturn_network_adapter.cpp index 6d7c1ab418a..38ce059964a 100644 --- a/src/playturn_network_adapter.cpp +++ b/src/playturn_network_adapter.cpp @@ -80,7 +80,7 @@ bool playturn_network_adapter::read(config& dst) assert(next_->cfg.all_children_count() > next_command_num_); config::all_children_iterator itor = child_old.ordered_begin(); //TODO: implement operator + (all_children_iterator, int ) properly - for(unsigned int i = 0; i < next_command_num_; i++) {itor++;} + std::advance(itor, next_command_num_); //TODO: implement a non const version of ordered children config& childchild_old = const_cast(itor->cfg); config& childchild = child.add_child(itor->key); From e8116089a2714b57b814540a62f1294d61215978 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:18:12 +0200 Subject: [PATCH 05/14] Pre instead of post increment a variable. Issue found by cppcheck. --- src/playturn_network_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playturn_network_adapter.cpp b/src/playturn_network_adapter.cpp index 38ce059964a..a9c1bac0598 100644 --- a/src/playturn_network_adapter.cpp +++ b/src/playturn_network_adapter.cpp @@ -86,7 +86,7 @@ bool playturn_network_adapter::read(config& dst) config& childchild = child.add_child(itor->key); childchild.swap(childchild_old); - next_command_num_++; + ++next_command_num_; if(next_->cfg.all_children_count() == next_command_num_) { next_command_num_ = 0; From b90fd14075a0efd79f92c574bd86242304730855 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:18:31 +0200 Subject: [PATCH 06/14] Pre instead of post increment a variable. Issue found by cppcheck. --- src/playturn_network_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playturn_network_adapter.cpp b/src/playturn_network_adapter.cpp index a9c1bac0598..0bcc74fbde3 100644 --- a/src/playturn_network_adapter.cpp +++ b/src/playturn_network_adapter.cpp @@ -90,7 +90,7 @@ bool playturn_network_adapter::read(config& dst) if(next_->cfg.all_children_count() == next_command_num_) { next_command_num_ = 0; - next_++; + ++next_; } return true; } From e381f564c5c7ae402d678870ce277777b7355ba9 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:18:38 +0200 Subject: [PATCH 07/14] Pre instead of post increment a variable. Issue found by cppcheck. --- src/playturn_network_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playturn_network_adapter.cpp b/src/playturn_network_adapter.cpp index 0bcc74fbde3..effa1682a31 100644 --- a/src/playturn_network_adapter.cpp +++ b/src/playturn_network_adapter.cpp @@ -97,7 +97,7 @@ bool playturn_network_adapter::read(config& dst) else { child.swap(child_old); - next_++; + ++next_; return true; } } From 4ca1afc4131292cc94bd95f3ead424a6123e8e28 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:22:04 +0200 Subject: [PATCH 08/14] Use empty() instead of comparing size() with 0. Issue found by cppcheck. --- src/playturn_network_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playturn_network_adapter.cpp b/src/playturn_network_adapter.cpp index effa1682a31..6f32a989018 100644 --- a/src/playturn_network_adapter.cpp +++ b/src/playturn_network_adapter.cpp @@ -14,7 +14,7 @@ static lg::log_domain log_network("network"); void playturn_network_adapter::read_from_network() { - assert(data_.size() > 0); + assert(!data_.empty()); this->data_.push_back(config()); config& back = data_.back(); From 35f0d7245469e90c83507f9f7431fadfec525025 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:23:58 +0200 Subject: [PATCH 09/14] Use empty() instead of comparing size() with 0. Issue found by cppcheck. --- src/playturn_network_adapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playturn_network_adapter.cpp b/src/playturn_network_adapter.cpp index 6f32a989018..2e555f11a84 100644 --- a/src/playturn_network_adapter.cpp +++ b/src/playturn_network_adapter.cpp @@ -48,7 +48,7 @@ void playturn_network_adapter::read_from_network() bool playturn_network_adapter::is_at_end() { - assert(data_.size() > 0); + assert(!data_.empty()); return this->next_ == data_.back().ordered_end(); } From b099e7c752d24898065d7b6d201e3b996c48f7b8 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:38:54 +0200 Subject: [PATCH 10/14] Use dtor + placement new in assignment operator. The change was used to simplify the code. A member was not copied and that was noticed by cppcheck. --- src/map_label.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/map_label.cpp b/src/map_label.cpp index a0233500d65..b23d8e28f42 100644 --- a/src/map_label.cpp +++ b/src/map_label.cpp @@ -60,11 +60,8 @@ map_labels::~map_labels() map_labels& map_labels::operator=(const map_labels& other) { if(this != &other) { - team_ = other.team_; - - config cfg; - other.write(cfg); - read(cfg); + this->~map_labels(); + new (this) map_labels(other); } return *this; } From 80d49c6011d3fb32dd8294328f5c32ec1798c7fa Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:48:52 +0200 Subject: [PATCH 11/14] Use a variable as const ref instead of const. Issue found by cppcheck. --- src/help.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/help.cpp b/src/help.cpp index 9776ea3c1de..101c890098e 100644 --- a/src/help.cpp +++ b/src/help.cpp @@ -1693,7 +1693,7 @@ public: if (info.union_type().size() == 1 && info.union_type()[0] == info.number() && info.is_nonnull()) { std::vector row; const std::string& name = info.name(); - const std::string id = info.id(); + const std::string& id = info.id(); const int moves = movement_type.movement_cost(terrain); const int views = movement_type.vision_cost(terrain); const int jams = movement_type.jamming_cost(terrain); From 0840f7627d05d42562fc9945a61117e31b39fa54 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:54:03 +0200 Subject: [PATCH 12/14] Limit variable scope. Issue found by cppcheck. --- src/race.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/race.cpp b/src/race.cpp index bcc4893da13..c7c7da96c3e 100644 --- a/src/race.cpp +++ b/src/race.cpp @@ -119,9 +119,8 @@ static ucs4::string markov_generate_name(const markov_prefix_map& prefixes, // markov prefix map. If no valid ending is found, use the // originally generated name. ucs4::string originalRes = res; - int prefixLen; while(!res.empty()) { - prefixLen = chain_size < res.size() ? chain_size : res.size(); + const int prefixLen = chain_size < res.size() ? chain_size : res.size(); prefix = ucs4::string(res.end() - prefixLen, res.end()); const markov_prefix_map::const_iterator i = prefixes.find(prefix); From a3c51d2f81ae17b3157911db30ed1ad67abd6223 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 20:58:24 +0200 Subject: [PATCH 13/14] Remove an unneeded helper variable. There was a cppcheck warning, that the scope of has_end_turn could be reduced. Removing the variable makes the even simpler. --- src/replay_controller.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/replay_controller.cpp b/src/replay_controller.cpp index 6ceca550a5b..94fd1f5cd9b 100644 --- a/src/replay_controller.cpp +++ b/src/replay_controller.cpp @@ -435,7 +435,6 @@ void replay_controller::play_side(const unsigned int /*team_index*/, bool){ try{ // If a side is empty skip over it. - bool has_end_turn = true; if (!current_team().is_empty()) { statistics::reset_turn_stats(current_team().save_id()); @@ -444,10 +443,8 @@ void replay_controller::play_side(const unsigned int /*team_index*/, bool){ DBG_REPLAY << "doing replay " << player_number_ << "\n"; // if have reached the end we don't want to execute finish_side_turn and finish_turn // becasue we might not have enough data to execute them (like advancements during turn_end for example) - // !has_end_turn == we reached the end of teh replay without finding and end turn tag. - has_end_turn = do_replay(player_number_) == REPLAY_FOUND_END_TURN; - if(!has_end_turn) - { + if(do_replay(player_number_) != REPLAY_FOUND_END_TURN) { + // We reached the end of teh replay without finding and end turn tag. return; } finish_side_turn(); From 2b943e2967b4374a0b5bdc5dd14062fbf3959bcd Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sat, 12 Apr 2014 21:03:58 +0200 Subject: [PATCH 14/14] Limit variable scope. Issue found by cppcheck. --- src/serialization/unicode.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/serialization/unicode.cpp b/src/serialization/unicode.cpp index 9a3a3ed4db2..da7c00665c5 100644 --- a/src/serialization/unicode.cpp +++ b/src/serialization/unicode.cpp @@ -276,9 +276,9 @@ size_t index(const utf8::string& str, const size_t index) { // chr counts characters, i is the codepoint index // remark: several functions rely on the fallback to str.length() - unsigned int chr, i = 0, len = str.size(); + unsigned int i = 0, len = str.size(); try { - for (chr=0; chr