[Scummvm-cvs-logs] scummvm master -> 417064e3116e9fbb5737f7f9f13acc54b914e7be

[lordhoto]

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:
ac4087856f ALL: Remove optimization unstable code on checking for null after new.
417064e311 Merge pull request #417 from digitall/STACK_fixes


Commit: ac4087856f02725c288e1cef6b089acf7a6121aa
    https://github.com/scummvm/scummvm/commit/ac4087856f02725c288e1cef6b089acf7a6121aa
Author: D G Turner (digitall at scummvm.org)
Date: 2014-01-14T18:36:19-08:00

Commit Message:
ALL: Remove optimization unstable code on checking for null after new.

These issues were identified by the STACK tool.

By default, the C++ new operator will throw an exception on allocation
failure, rather than returning a null pointer.

The result is that testing the returned pointer for null is redundant
and _may_ be removed by the compiler. This is thus optimization
unstable and may result in incorrect behaviour at runtime.

However, we do not use exceptions as they are not supported by all
compilers and may be disabled.

To make this stable without removing the null check, you could qualify
the new operator call with std::nothrow to indicate that this should
return a null, rather than throwing an exception.

However, using (std::nothrow) was not desirable due to the Symbian
toolchain lacking a <new> header.
A global solution to this was also not easy by redefining "new" as "new
(std::nothrow)" due to custom constructors in NDS toolchain and various
common classes.

Also, this would then need explicit checks for OOM adding to all new
usages as per C malloc which is untidy.

For now to remove this optimisation unstable code is best as it is
likely to not be present anyway, and OOM will cause a system library
exception instead, even without exceptions enabled in the application
code.

Changed paths:
    audio/decoders/voc.cpp
    common/quicktime.cpp
    engines/cge/cge_main.cpp
    engines/cge/sound.cpp
    engines/sci/graphics/picture.cpp
    engines/sci/graphics/ports.cpp
    engines/tony/sound.cpp
    graphics/sjis.cpp



diff --git a/audio/decoders/voc.cpp b/audio/decoders/voc.cpp
index fa330c6..ec58953 100644
--- a/audio/decoders/voc.cpp
+++ b/audio/decoders/voc.cpp
@@ -559,7 +559,7 @@ SeekableAudioStream *makeVOCStream(Common::SeekableReadStream *stream, byte flag
 
 	SeekableAudioStream *audioStream = new VocStream(stream, (flags & Audio::FLAG_UNSIGNED) != 0, disposeAfterUse);
 
-	if (audioStream && audioStream->endOfData()) {
+	if (audioStream->endOfData()) {
 		delete audioStream;
 		return 0;
 	} else {
diff --git a/common/quicktime.cpp b/common/quicktime.cpp
index a3efc2b..6ab5a42 100644
--- a/common/quicktime.cpp
+++ b/common/quicktime.cpp
@@ -368,9 +368,6 @@ int QuickTimeParser::readMVHD(Atom atom) {
 int QuickTimeParser::readTRAK(Atom atom) {
 	Track *track = new Track();
 
-	if (!track)
-		return -1;
-
 	track->codecType = CODEC_TYPE_MOV_OTHER;
 	track->startTime = 0; // XXX: check
 	_tracks.push_back(track);
diff --git a/engines/cge/cge_main.cpp b/engines/cge/cge_main.cpp
index 5325558..602b36d 100644
--- a/engines/cge/cge_main.cpp
+++ b/engines/cge/cge_main.cpp
@@ -539,14 +539,12 @@ void CGEEngine::setMapBrick(int x, int z) {
 	debugC(1, kCGEDebugEngine, "CGEEngine::setMapBrick(%d, %d)", x, z);
 
 	Square *s = new Square(this);
-	if (s) {
-		char n[6];
-		s->gotoxy(x * kMapGridX, kMapTop + z * kMapGridZ);
-		sprintf(n, "%02d:%02d", x, z);
-		_clusterMap[z][x] = 1;
-		s->setName(n);
-		_vga->_showQ->insert(s, _vga->_showQ->first());
-	}
+	char n[6];
+	s->gotoxy(x * kMapGridX, kMapTop + z * kMapGridZ);
+	sprintf(n, "%02d:%02d", x, z);
+	_clusterMap[z][x] = 1;
+	s->setName(n);
+	_vga->_showQ->insert(s, _vga->_showQ->first());
 }
 
 void CGEEngine::keyClick() {
diff --git a/engines/cge/sound.cpp b/engines/cge/sound.cpp
index b378898..892b826 100644
--- a/engines/cge/sound.cpp
+++ b/engines/cge/sound.cpp
@@ -186,6 +186,10 @@ DataCk *Fx::load(int idx, int ref) {
 
 DataCk *Fx::loadWave(EncryptedStream *file) {
 	byte *data = (byte *)malloc(file->size());
+
+	if (!data)
+		return 0;
+
 	file->read(data, file->size());
 
 	return new DataCk(data, file->size());
diff --git a/engines/sci/graphics/picture.cpp b/engines/sci/graphics/picture.cpp
index 374f720..caab1d8 100644
--- a/engines/sci/graphics/picture.cpp
+++ b/engines/sci/graphics/picture.cpp
@@ -283,8 +283,6 @@ void GfxPicture::drawCelData(byte *inbuffer, int size, int headerPos, int rlePos
 	//  That needs to be done cause a mirrored picture may be requested
 	pixelCount = width * height;
 	celBitmap = new byte[pixelCount];
-	if (!celBitmap)
-		error("Unable to allocate temporary memory for picture drawing");
 
 	if (g_sci->getPlatform() == Common::kPlatformMacintosh && getSciVersion() >= SCI_VERSION_2) {
 		// See GfxView::unpackCel() for why this black/white swap is done
diff --git a/engines/sci/graphics/ports.cpp b/engines/sci/graphics/ports.cpp
index 6d9dc03..ed0013b 100644
--- a/engines/sci/graphics/ports.cpp
+++ b/engines/sci/graphics/ports.cpp
@@ -300,11 +300,6 @@ Window *GfxPorts::addWindow(const Common::Rect &dims, const Common::Rect *restor
 	Window *pwnd = new Window(id);
 	Common::Rect r;
 
-	if (!pwnd) {
-		error("Can't open window");
-		return 0;
-	}
-
 	_windowsById[id] = pwnd;
 
 	// KQ1sci, KQ4, iceman, QfG2 always add windows to the back of the list.
diff --git a/engines/tony/sound.cpp b/engines/tony/sound.cpp
index 547f319..74d32c7 100644
--- a/engines/tony/sound.cpp
+++ b/engines/tony/sound.cpp
@@ -88,7 +88,7 @@ FPSound::~FPSound() {
 bool FPSound::createStream(FPStream **streamPtr) {
 	(*streamPtr) = new FPStream(_soundSupported);
 
-	return (*streamPtr != NULL);
+	return true;
 }
 
 /**
diff --git a/graphics/sjis.cpp b/graphics/sjis.cpp
index 33f0e56..59ee5af 100644
--- a/graphics/sjis.cpp
+++ b/graphics/sjis.cpp
@@ -40,31 +40,25 @@ FontSJIS *FontSJIS::createFont(const Common::Platform platform) {
 	// Try the font ROM of the specified platform
 	if (platform == Common::kPlatformFMTowns) {
 		ret = new FontTowns();
-		if (ret) {
-			if (ret->loadData())
-				return ret;
-		}
+		if (ret->loadData())
+			return ret;
 		delete ret;
 	} else if (platform == Common::kPlatformPCEngine) {
 		ret = new FontPCEngine();
-		if (ret) {
-			if (ret->loadData())
-				return ret;
-		}
+		if (ret->loadData())
+			return ret;
 		delete ret;
 	} // TODO: PC98 font rom support
 	/* else if (platform == Common::kPlatformPC98) {
 		ret = new FontPC98();
-		if (ret) {
-			if (ret->loadData())
-				return ret;
-		}
+		if (ret->loadData())
+			return ret;
 		delete ret;
 	}*/
 
 	// Try ScummVM's font.
 	ret = new FontSjisSVM(platform);
-	if (ret && ret->loadData())
+	if (ret->loadData())
 		return ret;
 	delete ret;
 


Commit: 417064e3116e9fbb5737f7f9f13acc54b914e7be
    https://github.com/scummvm/scummvm/commit/417064e3116e9fbb5737f7f9f13acc54b914e7be
Author: Johannes Schickel (lordhoto at gmail.com)
Date: 2014-01-17T17:48:16-08:00

Commit Message:
Merge pull request #417 from digitall/STACK_fixes

ALL: Fix optimization unstable code on checking for null after new.

Changed paths:
    audio/decoders/voc.cpp
    common/quicktime.cpp
    engines/cge/cge_main.cpp
    engines/cge/sound.cpp
    engines/sci/graphics/picture.cpp
    engines/sci/graphics/ports.cpp
    engines/tony/sound.cpp
    graphics/sjis.cpp