[Scummvm-git-logs] scummvm master -> 8b310f12710debaabc4850c2af8f9b87749fb4b4

[rvanlaar]

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:
9a6dea9466 DIRECTOR: Fix memory leaks in archive.cpp
8b310f1271 DIRECTOR: Fix memory leak in sound


Commit: 9a6dea94664abfa5f408b9414ca055ae741c8015
    https://github.com/scummvm/scummvm/commit/9a6dea94664abfa5f408b9414ca055ae741c8015
Author: Roland van Laar (roland at rolandvanlaar.nl)
Date: 2022-11-30T10:51:41+01:00

Commit Message:
DIRECTOR: Fix memory leaks in archive.cpp

The newly used Common::SeekableReadStreamEndianWrapper was leaking.
Setting the DisposeAfterUse to YES resolved the leaks.

Changed paths:
    engines/director/archive.cpp


diff --git a/engines/director/archive.cpp b/engines/director/archive.cpp
index bb22d5b3d3c..5878c8e0e52 100644
--- a/engines/director/archive.cpp
+++ b/engines/director/archive.cpp
@@ -120,7 +120,7 @@ Common::SeekableReadStreamEndian *Archive::getResource(uint32 tag, uint16 id) {
 	const Resource &res = resMap[id];
 	auto stream = new Common::SeekableSubReadStream(_stream, res.offset, res.offset + res.size, DisposeAfterUse::NO);
 
-	return new Common::SeekableReadStreamEndianWrapper(stream, _isBigEndian, DisposeAfterUse::NO);
+	return new Common::SeekableReadStreamEndianWrapper(stream, _isBigEndian, DisposeAfterUse::YES);
 }
 
 Resource Archive::getResourceDetail(uint32 tag, uint16 id) {
@@ -463,7 +463,7 @@ Common::SeekableReadStreamEndian *RIFFArchive::getResource(uint32 tag, uint16 id
 	debugC(4, kDebugLoading, "RIFFArchive::getResource() tag: %s id: %i offset: %i size: %i", tag2str(tag), id, res.offset, res.size);
 
 	auto stream = new Common::SeekableSubReadStream(_stream, _startOffset + offset, _startOffset + offset + size, DisposeAfterUse::NO);
-	return new Common::SeekableReadStreamEndianWrapper(stream, true, DisposeAfterUse::NO);
+	return new Common::SeekableReadStreamEndianWrapper(stream, true, DisposeAfterUse::YES);
 }
 
 // RIFX Archive code
@@ -948,7 +948,7 @@ Common::SeekableReadStreamEndian *RIFXArchive::getResource(uint32 tag, uint16 id
 	uint32 size = res.size;
 	auto stream = new Common::SeekableSubReadStream(_stream, offset, offset + size, DisposeAfterUse::NO);
 
-	return new Common::SeekableReadStreamEndianWrapper(stream, bigEndian, DisposeAfterUse::NO);
+	return new Common::SeekableReadStreamEndianWrapper(stream, bigEndian, DisposeAfterUse::YES);
 }
 
 Resource RIFXArchive::getResourceDetail(uint32 tag, uint16 id) {


Commit: 8b310f12710debaabc4850c2af8f9b87749fb4b4
    https://github.com/scummvm/scummvm/commit/8b310f12710debaabc4850c2af8f9b87749fb4b4
Author: Roland van Laar (roland at rolandvanlaar.nl)
Date: 2022-11-30T10:54:49+01:00

Commit Message:
DIRECTOR: Fix memory leak in sound

Delete the stream gotten from _macresman->getDataFork.

Made the stream copying more explicit by aligning the variables names
with the function logic.

Changed paths:
    engines/director/sound.cpp


diff --git a/engines/director/sound.cpp b/engines/director/sound.cpp
index 71d2e0422e4..1de5c9370fb 100644
--- a/engines/director/sound.cpp
+++ b/engines/director/sound.cpp
@@ -798,37 +798,44 @@ Audio::AudioStream *AudioFileDecoder::getAudioStream(bool looping, bool forPuppe
 		return nullptr;
 
 	Common::Path filePath = Common::Path(pathMakeRelative(_path), g_director->_dirSeparator);
-	Common::SeekableReadStream *file = nullptr;
+
+	Common::SeekableReadStream *dataFork = nullptr;
+	Common::SeekableReadStream *copiedStream = nullptr;
 
 	if (_macresman->open(filePath)) {
-		// Data has to be copied out instead of using the stream from
-		// getDataFork() directly because it's possible for this audio
-		// to outlive the owning MacResMan, which would otherwise free
-		// the stream while it's still being read from.
-		Common::SeekableReadStream *stream = _macresman->getDataFork();
-		file = stream->readStream(stream->size());
+
+		dataFork = _macresman->getDataFork();
 	}
 
-	if (file == nullptr) {
+	// Data has to be copied out instead of using the stream from
+	// getDataFork() directly because it's possible for this audio
+	// to outlive the owning MacResMan, which would otherwise free
+	// the stream while it's still being read from.
+	if (dataFork != nullptr)
+		copiedStream = dataFork->readStream(dataFork->size());
+	delete dataFork;
+
+	if (copiedStream == nullptr) {
 		warning("Failed to open %s", _path.c_str());
-		delete file;
+		delete copiedStream;
 		return nullptr;
 	}
-	uint32 magic1 = file->readUint32BE();
-	file->readUint32BE();
-	uint32 magic2 = file->readUint32BE();
-	file->seek(0);
+
+	uint32 magic1 = copiedStream->readUint32BE();
+	copiedStream->readUint32BE();
+	uint32 magic2 = copiedStream->readUint32BE();
+	copiedStream->seek(0);
 
 	Audio::RewindableAudioStream *stream = nullptr;
 	if (magic1 == MKTAG('R', 'I', 'F', 'F') &&
 		magic2 == MKTAG('W', 'A', 'V', 'E')) {
-		stream = Audio::makeWAVStream(file, disposeAfterUse);
+		stream = Audio::makeWAVStream(copiedStream, disposeAfterUse);
 	} else if (magic1 == MKTAG('F', 'O', 'R', 'M') &&
 				(magic2 == MKTAG('A', 'I', 'F', 'F') || magic2 == MKTAG('A', 'I', 'F', 'C'))) {
-		stream = Audio::makeAIFFStream(file, disposeAfterUse);
+		stream = Audio::makeAIFFStream(copiedStream, disposeAfterUse);
 	} else {
 		warning("Unknown file type for %s", _path.c_str());
-		delete file;
+		delete copiedStream;
 	}
 
 	if (stream) {