widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/lan-promotion-memory-leak into lp:widelands.

Commit message:
Fixed memory leak in LanGameFinder

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/lan-promotion-memory-leak/+merge/345338

To reproduce the bug:

1. Compile with ASan
2. Run Widelands and host a LAN game
3. Run a second instance of Widelands in a new terminal
4. Join the game and leave
5. Leave Widelands

-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/lan-promotion-memory-leak into lp:widelands.

=== modified file 'src/network/network_lan_promotion.cc'
--- src/network/network_lan_promotion.cc	2018-04-07 16:59:00 +0000
+++ src/network/network_lan_promotion.cc	2018-05-10 07:28:15 +0000
@@ -23,6 +23,8 @@
 #include <ifaddrs.h>
 #endif
 
+#include <memory>
+
 #include <boost/lexical_cast.hpp>
 
 #include "base/i18n.h"
@@ -467,30 +469,28 @@
 		//  if the game already is in the list, update the information
 		//  otherwise just append it to the list
 		bool was_in_list = false;
-		for (NetOpenGame* opengame : opengames) {
+		for (const auto& opengame : opengames) {
 			if (0 == strncmp(opengame->info.hostname, info.hostname, 128)) {
 				opengame->info = info;
 				if (!opengame->address.is_ipv6() && addr.is_ipv6()) {
 					opengame->address.ip = addr.ip;
 				}
-				callback(GameUpdated, opengame, userdata);
+				callback(GameUpdated, opengame.get(), userdata);
 				was_in_list = true;
 				break;
 			}
 		}
 
 		if (!was_in_list) {
-			opengames.push_back(new NetOpenGame);
 			addr.port = kWidelandsLanPort;
-			opengames.back()->address = addr;
-			opengames.back()->info = info;
-			callback(GameOpened, opengames.back(), userdata);
+			opengames.push_back(std::unique_ptr<NetOpenGame>(new NetOpenGame(addr, info)));
+			callback(GameOpened, opengames.back().get(), userdata);
 			break;
 		}
 	}
 }
 
-void LanGameFinder::set_callback(void (*const cb)(int32_t, NetOpenGame const*, void*),
+void LanGameFinder::set_callback(void (*const cb)(int32_t, const NetOpenGame* const, void*),
                                  void* const ud) {
 	callback = cb;
 	userdata = ud;

=== modified file 'src/network/network_lan_promotion.h'
--- src/network/network_lan_promotion.h	2018-04-07 16:59:00 +0000
+++ src/network/network_lan_promotion.h	2018-05-10 07:28:15 +0000
@@ -21,6 +21,7 @@
 #define WL_NETWORK_NETWORK_LAN_PROMOTION_H
 
 #include <list>
+#include <memory>
 #include <set>
 
 #include "network/network.h"
@@ -41,6 +42,9 @@
 };
 
 struct NetOpenGame {
+	NetOpenGame() = default;
+	explicit NetOpenGame(NetAddress init_address, NetGameInfo init_info) : address(init_address), info(init_info) {
+	}
 	NetAddress address;
 	NetGameInfo info;
 };
@@ -171,12 +175,12 @@
 	void reset();
 	void run();
 
-	void set_callback(void (*)(int32_t, NetOpenGame const*, void*), void*);
+	void set_callback(void (*)(int32_t, const NetOpenGame* const, void*), void*);
 
 private:
-	std::list<NetOpenGame*> opengames;
+	std::list<std::unique_ptr<NetOpenGame>> opengames;
 
-	void (*callback)(int32_t, NetOpenGame const*, void*);
+	void (*callback)(int32_t, const NetOpenGame* const, void*);
 	void* userdata;
 };