From ac4087856f02725c288e1cef6b089acf7a6121aa Mon Sep 17 00:00:00 2001 From: D G Turner Date: Sun, 15 Dec 2013 08:17:57 +0000 Subject: 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 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. --- audio/decoders/voc.cpp | 2 +- common/quicktime.cpp | 3 --- engines/cge/cge_main.cpp | 14 ++++++-------- engines/cge/sound.cpp | 4 ++++ engines/sci/graphics/picture.cpp | 2 -- engines/sci/graphics/ports.cpp | 5 ----- engines/tony/sound.cpp | 2 +- graphics/sjis.cpp | 20 +++++++------------- 8 files changed, 19 insertions(+), 33 deletions(-) diff --git a/audio/decoders/voc.cpp b/audio/decoders/voc.cpp index fa330c6f2c..ec589533eb 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 a3efc2b443..6ab5a42b89 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 5325558f8b..602b36d6ef 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 b378898955..892b826318 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 374f7208d8..caab1d80da 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 6d9dc03195..ed0013b491 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 547f31906e..74d32c7c0f 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 33f0e562cb..59ee5af8c4 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; -- cgit v1.2.3