widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20.

Commit message:
Fix crash in NetClient when host changes their mind about a custom map in mid-transfer.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1826669 in widelands: "R20-rc1 HeapUseAfterFree when changig Map during Download"
  https://bugs.launchpad.net/widelands/+bug/1826669

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1826669-mp-map-b20 into lp:widelands/build20.

=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc	2019-02-23 11:00:49 +0000
+++ src/network/gameclient.cc	2019-04-28 14:30:33 +0000
@@ -586,8 +586,7 @@
 		    d->settings.mapfilename.c_str());
 
 		// New map was set, so we clean up the buffer of a previously requested file
-		if (file_)
-			delete file_;
+		file_.reset(nullptr);
 		break;
 	}
 
@@ -637,10 +636,7 @@
 		s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
 		d->net->send(s);
 
-		if (file_)
-			delete file_;
-
-		file_ = new NetTransferFile();
+		file_.reset(new NetTransferFile());
 		file_->bytes = bytes;
 		file_->filename = path;
 		file_->md5sum = md5;

=== modified file 'src/network/gameclient.h'
--- src/network/gameclient.h	2019-02-23 11:00:49 +0000
+++ src/network/gameclient.h	2019-04-28 14:30:33 +0000
@@ -20,6 +20,8 @@
 #ifndef WL_NETWORK_GAMECLIENT_H
 #define WL_NETWORK_GAMECLIENT_H
 
+#include <memory>
+
 #include "chat/chat.h"
 #include "logic/game_controller.h"
 #include "logic/game_settings.h"
@@ -120,7 +122,7 @@
 	                bool sendreason = true,
 	                bool showmsg = true);
 
-	NetTransferFile* file_;
+	std::unique_ptr<NetTransferFile> file_;
 	GameClientImpl* d;
 	bool internet_;
 };

=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc	2019-02-23 11:00:49 +0000
+++ src/network/gamehost.cc	2019-04-28 14:30:33 +0000
@@ -524,7 +524,7 @@
 	hostuser.position = UserSettings::none();
 	hostuser.ready = true;
 	d->settings.users.push_back(hostuser);
-	file_ = nullptr;  //  Initialize as 0 pointer - unfortunately needed in struct.
+	file_.reset(nullptr);  //  Initialize as 0 pointer - unfortunately needed in struct.
 }
 
 GameHost::~GameHost() {
@@ -539,7 +539,6 @@
 	d->net.reset();
 	d->promoter.reset();
 	delete d;
-	delete file_;
 }
 
 const std::string& GameHost::get_local_playername() const {
@@ -1076,9 +1075,7 @@
 		// Read in the file
 		FileRead fr;
 		fr.open(*g_fs, mapfilename);
-		if (file_)
-			delete file_;
-		file_ = new NetTransferFile();
+		file_.reset(new NetTransferFile());
 		file_->filename = mapfilename;
 		uint32_t leftparts = file_->bytes = fr.get_size();
 		while (leftparts > 0) {
@@ -1097,10 +1094,7 @@
 		file_->md5sum = md5sum.get_checksum().str();
 	} else {
 		// reset previously offered map / saved game
-		if (file_) {
-			delete file_;
-			file_ = nullptr;
-		}
+		file_.reset(nullptr);
 	}
 
 	packet.reset();

=== modified file 'src/network/gamehost.h'
--- src/network/gamehost.h	2019-02-23 11:00:49 +0000
+++ src/network/gamehost.h	2019-04-28 14:30:33 +0000
@@ -20,6 +20,8 @@
 #ifndef WL_NETWORK_GAMEHOST_H
 #define WL_NETWORK_GAMEHOST_H
 
+#include <memory>
+
 #include "logic/game_controller.h"
 #include "logic/game_settings.h"
 #include "logic/player_end_result.h"
@@ -156,7 +158,7 @@
 	                       const std::string& arg = "");
 	void reaper();
 
-	NetTransferFile* file_;
+	std::unique_ptr<NetTransferFile> file_;
 	GameHostImpl* d;
 	bool internet_;
 	bool forced_pause_;

=== modified file 'src/network/network.h'
--- src/network/network.h	2019-02-23 11:00:49 +0000
+++ src/network/network.h	2019-04-28 14:30:33 +0000
@@ -181,6 +181,11 @@
 };
 
 struct NetTransferFile {
+	NetTransferFile() : bytes(0), filename(""), md5sum("") {
+	}
+	~NetTransferFile() {
+		parts.clear();
+	}
 	uint32_t bytes;
 	std::string filename;
 	std::string md5sum;