Merge lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange into lp:widelands/build20

Proposed by Klaus Halfmann
Status: Rejected
Rejected by: Toni Förster
Proposed branch: lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange
Merge into: lp:widelands/build20
Diff against target: 60 lines (+16/-3)
3 files modified
src/network/gameclient.cc (+6/-3)
src/network/gamehost.cc (+2/-0)
src/network/network.h (+8/-0)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug_1826669_MapDownloandChange
Reviewer Review Type Date Requested Status
Toni Förster testing, playing, compiling Approve
Review via email: mp+366616@code.launchpad.net

Commit message

Fix for #1826669

Description of the change

Avoid double delete of _file in gameclient.cc

To post a comment you must log in.
Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

This will merge to buld20.

I will try to do a bigger refactoring for trunk, but we should merge it there, too.

Revision history for this message
Toni Förster (stonerl) wrote :

Tested with Klaus and this fixes the crash.

review: Approve (testing, playing, compiling)
Revision history for this message
GunChleoc (gunchleoc) wrote :

I have an alternative fix that will use unique_ptr. We are trying to get away from raw delete calls. Tested with 2 Ubuntu machines.

https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366617

Revision history for this message
GunChleoc (gunchleoc) wrote :
Revision history for this message
GunChleoc (gunchleoc) wrote :

Woohoo, forgot to tick the checkbox too. Now I finally have the correct target branch: https://code.launchpad.net/~widelands-dev/widelands/bug-1826669-mp-map-b20/+merge/366619

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

Shouldn't this merge request being dropped an th branch abandoned since it was fixed by another branch?

Revision history for this message
Toni Förster (stonerl) wrote :

Set to rejected since we already have merged Gun's branch.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc 2019-02-23 11:00:49 +0000
+++ src/network/gameclient.cc 2019-04-28 12:08:11 +0000
@@ -586,8 +586,10 @@
586 d->settings.mapfilename.c_str());586 d->settings.mapfilename.c_str());
587587
588 // New map was set, so we clean up the buffer of a previously requested file588 // New map was set, so we clean up the buffer of a previously requested file
589 if (file_)589 if (file_) {
590 delete file_;590 delete file_;
591 file_ = NULL;
592 }
591 break;593 break;
592 }594 }
593595
@@ -637,8 +639,9 @@
637 s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);639 s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
638 d->net->send(s);640 d->net->send(s);
639641
640 if (file_)642 if (file_) {
641 delete file_;643 delete file_;
644 }
642645
643 file_ = new NetTransferFile();646 file_ = new NetTransferFile();
644 file_->bytes = bytes;647 file_->bytes = bytes;
645648
=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc 2019-02-23 11:00:49 +0000
+++ src/network/gamehost.cc 2019-04-28 12:08:11 +0000
@@ -539,7 +539,9 @@
539 d->net.reset();539 d->net.reset();
540 d->promoter.reset();540 d->promoter.reset();
541 delete d;541 delete d;
542 d = NULL;
542 delete file_;543 delete file_;
544 file_ = NULL;
543}545}
544546
545const std::string& GameHost::get_local_playername() const {547const std::string& GameHost::get_local_playername() const {
546548
=== modified file 'src/network/network.h'
--- src/network/network.h 2019-02-23 11:00:49 +0000
+++ src/network/network.h 2019-04-28 12:08:11 +0000
@@ -185,6 +185,14 @@
185 std::string filename;185 std::string filename;
186 std::string md5sum;186 std::string md5sum;
187 std::vector<FilePart> parts;187 std::vector<FilePart> parts;
188
189 NetTransferFile() {
190 // Allow Debugging
191 }
192
193 ~NetTransferFile() {
194 // Allow Debugging
195 }
188};196};
189197
190class Deserializer {198class Deserializer {

Subscribers

People subscribed via source and target branches

to all changes: