[Scummvm-git-logs] scummvm master -> 46fe41a77c6cfb4ed4976b3e13cb8beea3098cee

sev- noreply at scummvm.org
Sun Sep 25 08:35:09 UTC 2022


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

Summary:
f502638290 DIRECTOR: Plug file handle memory leaks
4e22e09685 DIRECTOR: Prevent CastData leak if multiple with the same ID
00a7d69fd1 DIRECTOR: Plug stream leak for Mac archives
095ae06405 COMMON: Refactor MacResManager::load* methods to accept a pointer
15a9cbe002 DIRECTOR: LINGO: Fix Datum leak in c_starts
46fe41a77c DIRECTOR: Plug memory leak in AVI loader


Commit: f502638290e7345672f159f692daedc12a89eeb5
    https://github.com/scummvm/scummvm/commit/f502638290e7345672f159f692daedc12a89eeb5
Author: Scott Percival (code at moral.net.au)
Date: 2022-09-25T10:35:03+02:00

Commit Message:
DIRECTOR: Plug file handle memory leaks

Changed paths:
    engines/director/lingo/lingo.cpp
    engines/director/resource.cpp


diff --git a/engines/director/lingo/lingo.cpp b/engines/director/lingo/lingo.cpp
index 820dfca45d9..bb6843218f5 100644
--- a/engines/director/lingo/lingo.cpp
+++ b/engines/director/lingo/lingo.cpp
@@ -1294,10 +1294,9 @@ void Lingo::runTests() {
 			}
 
 			free(script);
-
+			delete stream;
 			counter++;
 		}
-		delete stream;
 
 		inFile.close();
 	}
diff --git a/engines/director/resource.cpp b/engines/director/resource.cpp
index a855ffb70ce..c52a637f2fa 100644
--- a/engines/director/resource.cpp
+++ b/engines/director/resource.cpp
@@ -85,6 +85,7 @@ Common::Error Window::loadInitialMovie() {
 			_currentMovie->processEvent(kEventStartUp);
 
 			free(script);
+			delete stream;
 		} else {
 			warning("Window::LoadInitialMovie: failed to load startup scripts");
 		}
@@ -212,10 +213,10 @@ void Window::loadEXE(const Common::String movie) {
 		_currentMovie = nullptr;
 
 		free(script);
+		delete iniStream;
 	} else {
 		warning("No LINGO.INI");
 	}
-	delete iniStream;
 
 	Common::SeekableReadStream *exeStream = SearchMan.createReadStreamForMember(Common::Path(movie, g_director->_dirSeparator));
 	if (!exeStream)


Commit: 4e22e09685ab602a69abd5a29740bb4bd8a943f0
    https://github.com/scummvm/scummvm/commit/4e22e09685ab602a69abd5a29740bb4bd8a943f0
Author: Scott Percival (code at moral.net.au)
Date: 2022-09-25T10:35:03+02:00

Commit Message:
DIRECTOR: Prevent CastData leak if multiple with the same ID

Changed paths:
    engines/director/cast.cpp


diff --git a/engines/director/cast.cpp b/engines/director/cast.cpp
index 3a3a7695205..85ef1ff0cb5 100644
--- a/engines/director/cast.cpp
+++ b/engines/director/cast.cpp
@@ -1064,6 +1064,12 @@ void Cast::loadCastData(Common::SeekableReadStreamEndian &stream, uint16 id, Res
 
 	Common::MemoryReadStreamEndian castStream(data, castSizeToRead, stream.isBE());
 
+	if (_loadedCast->contains(id)) {
+		warning("Cast::loadCastData(): Multiple cast members with ID %d, overwriting", id);
+		delete _loadedCast->getVal(id);
+		_loadedCast->erase(id);
+	}
+
 	switch (castType) {
 	case kCastBitmap:
 		debugC(3, kDebugLoading, "Cast::loadCastData(): loading kCastBitmap (%d children)", res->children.size());


Commit: 00a7d69fd1926b8e28ca36898d8161149c1f9a8b
    https://github.com/scummvm/scummvm/commit/00a7d69fd1926b8e28ca36898d8161149c1f9a8b
Author: Scott Percival (code at moral.net.au)
Date: 2022-09-25T10:35:03+02:00

Commit Message:
DIRECTOR: Plug stream leak for Mac archives

Changed paths:
    engines/director/archive.cpp


diff --git a/engines/director/archive.cpp b/engines/director/archive.cpp
index 4d7e5cb286a..e8b1731c5d5 100644
--- a/engines/director/archive.cpp
+++ b/engines/director/archive.cpp
@@ -496,6 +496,7 @@ bool RIFXArchive::openStream(Common::SeekableReadStream *stream, uint32 startOff
 			MacArchive *macArchive = new MacArchive();
 			if (!macArchive->openStream(macStream)) {
 				delete macArchive;
+				delete macStream;
 			} else {
 				g_director->getCurrentWindow()->probeMacBinary(macArchive);
 			}


Commit: 095ae06405bf618d628b5ee0fa069a0e92ee78c7
    https://github.com/scummvm/scummvm/commit/095ae06405bf618d628b5ee0fa069a0e92ee78c7
Author: Scott Percival (code at moral.net.au)
Date: 2022-09-25T10:35:03+02:00

Commit Message:
COMMON: Refactor MacResManager::load* methods to accept a pointer

Previously the methods would pass by reference, then convert to a
pointer on success and take ownership. This isn't ideal, as a
reference suggests no transferral of ownership, and doesn't suggest
it needs to be a heap resource allocated by new.

Changed paths:
    common/macresman.cpp
    common/macresman.h
    engines/director/archive.cpp
    graphics/macgui/macfontmanager.cpp


diff --git a/common/macresman.cpp b/common/macresman.cpp
index 676e4c0e128..706fa183af9 100644
--- a/common/macresman.cpp
+++ b/common/macresman.cpp
@@ -143,7 +143,7 @@ bool MacResManager::open(const Path &fileName, Archive &archive) {
 		String fullPath = plainFsNode->getPath() + "/..namedfork/rsrc";
 		FSNode resFsNode = FSNode(fullPath);
 		SeekableReadStream *macResForkRawStream = resFsNode.createReadStream();
-		if (!isMacBinaryFile && macResForkRawStream && loadFromRawFork(*macResForkRawStream)) {
+		if (!isMacBinaryFile && macResForkRawStream && loadFromRawFork(macResForkRawStream)) {
 			_baseFileName = fileName;
 			return true;
 		}
@@ -160,12 +160,12 @@ bool MacResManager::open(const Path &fileName, Archive &archive) {
 		bool appleDouble = (stream->readUint32BE() == 0x00051607);
 		stream->seek(0);
 
-		if (appleDouble && loadFromAppleDouble(*stream)) {
+		if (appleDouble && loadFromAppleDouble(stream)) {
 			_baseFileName = fileName;
 			return true;
 		}
 
-		if (loadFromRawFork(*stream)) {
+		if (loadFromRawFork(stream)) {
 			_baseFileName = fileName;
 			return true;
 		}
@@ -174,7 +174,7 @@ bool MacResManager::open(const Path &fileName, Archive &archive) {
 
 	// Then try for AppleDouble using Apple's naming
 	stream = archive.createReadStreamForMember(constructAppleDoubleName(fileName));
-	if (stream && loadFromAppleDouble(*stream)) {
+	if (stream && loadFromAppleDouble(stream)) {
 		_baseFileName = fileName;
 		return true;
 	}
@@ -182,7 +182,7 @@ bool MacResManager::open(const Path &fileName, Archive &archive) {
 
 	// Check .bin for MacBinary next
 	stream = archive.createReadStreamForMember(fileName.append(".bin"));
-	if (stream && loadFromMacBinary(*stream)) {
+	if (stream && loadFromMacBinary(stream)) {
 		_baseFileName = fileName;
 		return true;
 	}
@@ -197,7 +197,7 @@ bool MacResManager::open(const Path &fileName, Archive &archive) {
 		// Check it here
 		if (isMacBinary(*stream)) {
 			stream->seek(0);
-			if (loadFromMacBinary(*stream))
+			if (loadFromMacBinary(stream))
 				return true;
 		}
 
@@ -301,18 +301,21 @@ void MacResManager::listFiles(StringArray &files, const String &pattern) {
 	}
 }
 
-bool MacResManager::loadFromAppleDouble(SeekableReadStream &stream) {
-	if (stream.readUint32BE() != 0x00051607) // tag
+bool MacResManager::loadFromAppleDouble(SeekableReadStream *stream) {
+	if (!stream)
 		return false;
 
-	stream.skip(20); // version + home file system
+	if (stream->readUint32BE() != 0x00051607) // tag
+		return false;
+
+	stream->skip(20); // version + home file system
 
-	uint16 entryCount = stream.readUint16BE();
+	uint16 entryCount = stream->readUint16BE();
 
 	for (uint16 i = 0; i < entryCount; i++) {
-		uint32 id = stream.readUint32BE();
-		uint32 offset = stream.readUint32BE();
-		uint32 length = stream.readUint32BE(); // length
+		uint32 id = stream->readUint32BE();
+		uint32 offset = stream->readUint32BE();
+		uint32 length = stream->readUint32BE(); // length
 
 		if (id == 2) {
 			// Found the resource fork!
@@ -376,9 +379,12 @@ bool MacResManager::isRawFork(SeekableReadStream &stream) {
 	       && mapOffset < (uint32)stream.size() && mapOffset + mapLength <= (uint32)stream.size();
 }
 
-bool MacResManager::loadFromMacBinary(SeekableReadStream &stream) {
+bool MacResManager::loadFromMacBinary(SeekableReadStream *stream) {
+	if (!stream)
+		return false;
+
 	byte infoHeader[MBI_INFOHDR];
-	stream.read(infoHeader, MBI_INFOHDR);
+	stream->read(infoHeader, MBI_INFOHDR);
 
 	// Maybe we have MacBinary?
 	if (infoHeader[MBI_ZERO1] == 0 && infoHeader[MBI_ZERO2] == 0 &&
@@ -393,7 +399,7 @@ bool MacResManager::loadFromMacBinary(SeekableReadStream &stream) {
 		//uint32 rsrcSizePad = (((rsrcSize + 127) >> 7) << 7);
 
 		// Length check
-		if (MBI_INFOHDR + dataSizePad + rsrcSize <= (uint32)stream.size()) {
+		if (MBI_INFOHDR + dataSizePad + rsrcSize <= (uint32)stream->size()) {
 			_resForkOffset = MBI_INFOHDR + dataSizePad;
 			_resForkSize = rsrcSize;
 		}
@@ -406,27 +412,33 @@ bool MacResManager::loadFromMacBinary(SeekableReadStream &stream) {
 	return load(stream);
 }
 
-bool MacResManager::loadFromRawFork(SeekableReadStream &stream) {
+bool MacResManager::loadFromRawFork(SeekableReadStream *stream) {
+	if (!stream)
+		return false;
+
 	_mode = kResForkRaw;
 	_resForkOffset = 0;
-	_resForkSize = stream.size();
+	_resForkSize = stream->size();
 	return load(stream);
 }
 
-bool MacResManager::load(SeekableReadStream &stream) {
+bool MacResManager::load(SeekableReadStream *stream) {
+	if (!stream)
+		return false;
+
 	if (_mode == kResForkNone)
 		return false;
 
-	stream.seek(_resForkOffset);
+	stream->seek(_resForkOffset);
 
-	_dataOffset = stream.readUint32BE() + _resForkOffset;
-	_mapOffset = stream.readUint32BE() + _resForkOffset;
-	_dataLength = stream.readUint32BE();
-	_mapLength = stream.readUint32BE();
+	_dataOffset = stream->readUint32BE() + _resForkOffset;
+	_mapOffset = stream->readUint32BE() + _resForkOffset;
+	_dataLength = stream->readUint32BE();
+	_mapLength = stream->readUint32BE();
 
 	// do sanity check
-	if (stream.eos() || _dataOffset >= (uint32)stream.size() || _mapOffset >= (uint32)stream.size() ||
-			_dataLength + _mapLength  > (uint32)stream.size()) {
+	if (stream->eos() || _dataOffset >= (uint32)stream->size() || _mapOffset >= (uint32)stream->size() ||
+			_dataLength + _mapLength  > (uint32)stream->size()) {
 		_resForkOffset = -1;
 		_mode = kResForkNone;
 		return false;
@@ -435,7 +447,7 @@ bool MacResManager::load(SeekableReadStream &stream) {
 	debug(7, "got header: data %d [%d] map %d [%d]",
 		_dataOffset, _dataLength, _mapOffset, _mapLength);
 
-	_stream = &stream;
+	_stream = stream;
 
 	readMap();
 	return true;
diff --git a/common/macresman.h b/common/macresman.h
index 5e9d7cec8e2..50fed875f41 100644
--- a/common/macresman.h
+++ b/common/macresman.h
@@ -195,7 +195,7 @@ public:
 	/**
 	 * Load from stream in MacBinary format
 	 */
-	bool loadFromMacBinary(SeekableReadStream &stream);
+	bool loadFromMacBinary(SeekableReadStream *stream);
 
 	/**
 	 * Dump contents of the archive to ./dumps directory
@@ -224,10 +224,10 @@ private:
 	SeekableReadStream *_stream;
 	Path _baseFileName;
 
-	bool load(SeekableReadStream &stream);
+	bool load(SeekableReadStream *stream);
 
-	bool loadFromRawFork(SeekableReadStream &stream);
-	bool loadFromAppleDouble(SeekableReadStream &stream);
+	bool loadFromRawFork(SeekableReadStream *stream);
+	bool loadFromAppleDouble(SeekableReadStream *stream);
 
 	static Path constructAppleDoubleName(Path name);
 	static Path disassembleAppleDoubleName(Path name, bool *isAppleDouble);
diff --git a/engines/director/archive.cpp b/engines/director/archive.cpp
index e8b1731c5d5..ebf64f430d6 100644
--- a/engines/director/archive.cpp
+++ b/engines/director/archive.cpp
@@ -292,7 +292,7 @@ bool MacArchive::openStream(Common::SeekableReadStream *stream, uint32 startOffs
 	_resFork = new Common::MacResManager();
 	stream->seek(startOffset);
 
-	if (!_resFork->loadFromMacBinary(*stream)) {
+	if (!_resFork->loadFromMacBinary(stream)) {
 		warning("MacArchive::openStream(): Error loading Mac Binary");
 		close();
 		return false;
diff --git a/graphics/macgui/macfontmanager.cpp b/graphics/macgui/macfontmanager.cpp
index cd1b159d9e8..667b9a8a633 100644
--- a/graphics/macgui/macfontmanager.cpp
+++ b/graphics/macgui/macfontmanager.cpp
@@ -339,7 +339,7 @@ void MacFontManager::loadJapaneseFonts() {
 void MacFontManager::loadFonts(Common::SeekableReadStream *stream) {
 	Common::MacResManager fontFile;
 
-	if (!fontFile.loadFromMacBinary(*stream))
+	if (!fontFile.loadFromMacBinary(stream))
 		return;
 
 	loadFonts(&fontFile);


Commit: 15a9cbe0026ba8a7f9397895255a32eb1bb6843c
    https://github.com/scummvm/scummvm/commit/15a9cbe0026ba8a7f9397895255a32eb1bb6843c
Author: Scott Percival (code at moral.net.au)
Date: 2022-09-25T10:35:03+02:00

Commit Message:
DIRECTOR: LINGO: Fix Datum leak in c_starts

Changed paths:
    engines/director/lingo/lingo-code.cpp


diff --git a/engines/director/lingo/lingo-code.cpp b/engines/director/lingo/lingo-code.cpp
index 11d002d6adc..c07dc25fba9 100644
--- a/engines/director/lingo/lingo-code.cpp
+++ b/engines/director/lingo/lingo-code.cpp
@@ -940,10 +940,7 @@ void LC::c_starts() {
 
 	int res = s1.hasPrefix(s2) ? 1 : 0;
 
-	d1.type = INT;
-	d1.u.i = res;
-
-	g_lingo->push(d1);
+	g_lingo->push(Datum(res));
 }
 
 void LC::c_intersects() {


Commit: 46fe41a77c6cfb4ed4976b3e13cb8beea3098cee
    https://github.com/scummvm/scummvm/commit/46fe41a77c6cfb4ed4976b3e13cb8beea3098cee
Author: Scott Percival (code at moral.net.au)
Date: 2022-09-25T10:35:03+02:00

Commit Message:
DIRECTOR: Plug memory leak in AVI loader

Changed paths:
    engines/director/castmember.cpp
    video/video_decoder.cpp


diff --git a/engines/director/castmember.cpp b/engines/director/castmember.cpp
index 5a8573c7509..82372265dac 100644
--- a/engines/director/castmember.cpp
+++ b/engines/director/castmember.cpp
@@ -735,6 +735,11 @@ bool DigitalVideoCastMember::loadVideo(Common::String path) {
 		delete _video;
 		_video = new Video::AVIDecoder();
 		result = _video->loadFile(Common::Path(path1, g_director->_dirSeparator));
+		if (!result) {
+		    warning("DigitalVideoCastMember::loadVideo(): format not supported, skipping");
+		    delete _video;
+		    _video = nullptr;
+		}
 	}
 
 	if (result && g_director->_pixelformat.bytesPerPixel == 1) {
diff --git a/video/video_decoder.cpp b/video/video_decoder.cpp
index 9feb1de843f..841e6637be6 100644
--- a/video/video_decoder.cpp
+++ b/video/video_decoder.cpp
@@ -89,7 +89,10 @@ bool VideoDecoder::loadFile(const Common::Path &filename) {
 		return false;
 	}
 
-	return loadStream(file);
+	bool result = loadStream(file);
+	if (!result)
+		delete file;
+	return result;
 }
 
 bool VideoDecoder::needsUpdate() const {




More information about the Scummvm-git-logs mailing list