Skip to content

Commit

Permalink
ALL: Remove optimization unstable code on checking for null after new.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
digitall committed Jan 15, 2014
1 parent 74fbd2d commit ac40878
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 33 deletions.
2 changes: 1 addition & 1 deletion audio/decoders/voc.cpp
Expand Up @@ -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 {
Expand Down
3 changes: 0 additions & 3 deletions common/quicktime.cpp
Expand Up @@ -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);
Expand Down
14 changes: 6 additions & 8 deletions engines/cge/cge_main.cpp
Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions engines/cge/sound.cpp
Expand Up @@ -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());
Expand Down
2 changes: 0 additions & 2 deletions engines/sci/graphics/picture.cpp
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions engines/sci/graphics/ports.cpp
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion engines/tony/sound.cpp
Expand Up @@ -88,7 +88,7 @@ FPSound::~FPSound() {
bool FPSound::createStream(FPStream **streamPtr) {
(*streamPtr) = new FPStream(_soundSupported);

return (*streamPtr != NULL);
return true;
}

/**
Expand Down
20 changes: 7 additions & 13 deletions graphics/sjis.cpp
Expand Up @@ -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;

Expand Down

0 comments on commit ac40878

Please sign in to comment.