[Scummvm-git-logs] scummvm branch-2-3 -> 4f20e40409569475cfb325ba506d9b3b7bc6068e

[sev-]

This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
92a2519846 COMMON: Fix _disposeFileHandle not being set
4f20e40409 COMMON: Fix Valgrind warning in PEResources


Commit: 92a2519846d537d6a75c09f239c1ebf5118a4e3a
    https://github.com/scummvm/scummvm/commit/92a2519846d537d6a75c09f239c1ebf5118a4e3a
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-09-06T19:25:59+02:00

Commit Message:
COMMON: Fix _disposeFileHandle not being set

If there is a file handle, the clear() method checks if it should be
disposed of. However, the _disposeFileHandle variable was never set, so
it's unlikely that _exe was ever deleted. I found this out from a
Valgrind warning when quitting Buried in Time.

This seems like a very obvious fix to me, and as such it seems like a
good candidate for backporting to the release branch. On the other hand,
maybe there are cases where it worked by sheer, dumb luck? I'm not
familiar enough with where and how it is used.

Changed paths:
    common/winexe_ne.cpp


diff --git a/common/winexe_ne.cpp b/common/winexe_ne.cpp
index c81b16c76e..0f5ec73c26 100644
--- a/common/winexe_ne.cpp
+++ b/common/winexe_ne.cpp
@@ -52,6 +52,7 @@ bool NEResources::loadFromEXE(SeekableReadStream *stream, DisposeAfterUse::Flag
 		return false;
 
 	_exe = stream;
+	_disposeFileHandle = disposeFileHandle;
 
 	uint32 offsetResourceTable = getResourceTableOffset();
 	if (offsetResourceTable == 0xFFFFFFFF)


Commit: 4f20e40409569475cfb325ba506d9b3b7bc6068e
    https://github.com/scummvm/scummvm/commit/4f20e40409569475cfb325ba506d9b3b7bc6068e
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-09-06T19:26:03+02:00

Commit Message:
COMMON: Fix Valgrind warning in PEResources

As with NEResources, _disposeFileHandle was never set.

Changed paths:
    common/winexe_pe.cpp


diff --git a/common/winexe_pe.cpp b/common/winexe_pe.cpp
index 48989694de..13a8159830 100644
--- a/common/winexe_pe.cpp
+++ b/common/winexe_pe.cpp
@@ -40,9 +40,11 @@ PEResources::~PEResources() {
 void PEResources::clear() {
 	_sections.clear();
 	_resources.clear();
-	if (_disposeFileHandle == DisposeAfterUse::YES)
-		delete _exe;
-	_exe = nullptr;
+	if (_exe) {
+		if (_disposeFileHandle == DisposeAfterUse::YES)
+			delete _exe;
+		_exe = nullptr;
+	}
 }
 
 bool PEResources::loadFromEXE(SeekableReadStream *stream, DisposeAfterUse::Flag disposeFileHandle) {
@@ -95,6 +97,7 @@ bool PEResources::loadFromEXE(SeekableReadStream *stream, DisposeAfterUse::Flag
 	}
 
 	_exe = stream;
+	_disposeFileHandle = disposeFileHandle;
 
 	Section &resSection = _sections[".rsrc"];
 	parseResourceLevel(resSection, resSection.offset, 0);