[Scummvm-git-logs] scummvm master -> 8bbc6c62474a8ba25cf53e67c443029e66db90d3
AndywinXp
noreply at scummvm.org
Wed Jun 18 16:35:21 UTC 2025
This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://api.github.com/repos/scummvm/scummvm .
Summary:
8bbc6c6247 LASTEXPRESS: Fix vast majority of Coverity issues
Commit: 8bbc6c62474a8ba25cf53e67c443029e66db90d3
https://github.com/scummvm/scummvm/commit/8bbc6c62474a8ba25cf53e67c443029e66db90d3
Author: AndywinXp (andywinxp at gmail.com)
Date: 2025-06-18T18:35:14+02:00
Commit Message:
LASTEXPRESS: Fix vast majority of Coverity issues
Most of them were intended behavior or leftover debug paths...
It also uncovered some disasm clean-up mistakes I made though.
Changed paths:
engines/lastexpress/characters/anna.cpp
engines/lastexpress/characters/tatiana.cpp
engines/lastexpress/debug.cpp
engines/lastexpress/fight/fighter.cpp
engines/lastexpress/lastexpress.cpp
engines/lastexpress/memory.cpp
engines/lastexpress/sound/mixer.cpp
engines/lastexpress/sound/sound.cpp
diff --git a/engines/lastexpress/characters/anna.cpp b/engines/lastexpress/characters/anna.cpp
index 9b1a210a75e..399737429bd 100644
--- a/engines/lastexpress/characters/anna.cpp
+++ b/engines/lastexpress/characters/anna.cpp
@@ -1481,9 +1481,9 @@ void LogicManager::HAND_Anna_FreshenUp(HAND_PARAMS) {
if (getCharacter(kCharacterAnna).callbacks[getCharacter(kCharacterAnna).currentCall + 8] == 1 ||
getCharacter(kCharacterAnna).callbacks[getCharacter(kCharacterAnna).currentCall + 8] == 2) {
if (_globals[kProgressField14] == 29) {
- getCharacter(kCharacterAnna).callbacks[getCharacter(kCharacterAnna).currentCall + 8] = _gameTime + 900;
+ getCharacterCurrentParams(kCharacterAnna)[0] = _gameTime + 900;
getCharacter(kCharacterAnna).callbacks[getCharacter(kCharacterAnna).currentCall + 8] = 2;
- AnnaCall(&LogicManager::CONS_Anna_CompLogic, getCharacter(kCharacterAnna).callbacks[getCharacter(kCharacterAnna).currentCall + 8], "NONE", 0, 0);
+ AnnaCall(&LogicManager::CONS_Anna_CompLogic, getCharacterCurrentParams(kCharacterAnna)[0], "NONE", 0, 0);
} else {
getCharacter(kCharacterAnna).callbacks[getCharacter(kCharacterAnna).currentCall + 8] = 3;
AnnaCall(&LogicManager::CONS_Anna_DoCorrOtis, "618Bf", 37, 0, 0);
diff --git a/engines/lastexpress/characters/tatiana.cpp b/engines/lastexpress/characters/tatiana.cpp
index ee518f14380..e57c096c715 100644
--- a/engines/lastexpress/characters/tatiana.cpp
+++ b/engines/lastexpress/characters/tatiana.cpp
@@ -2603,9 +2603,12 @@ void LogicManager::HAND_Tatiana_SeekCath(HAND_PARAMS) {
if (!getCharacterCurrentParams(kCharacterTatiana)[0]) {
if (_doneNIS[kEventTatianaTylerCompartment] || _gameTime > 2475000) {
- if (getCharacterCurrentParams(kCharacterTatiana)[0]) {
- setDoor(1, 0, checkDoor(1), 10, 9);
- }
+ // Dead-code, byproduct of unrolling a goto...
+ //
+ // if (getCharacterCurrentParams(kCharacterTatiana)[0]) {
+ // setDoor(1, kCharacterCath, checkDoor(1), 10, 9);
+ // }
+
_globals[kProgressFieldE4] = 0;
softReleaseAtDoor(kCharacterTatiana, 2);
getCharacter(kCharacterTatiana).callbacks[getCharacter(kCharacterTatiana).currentCall + 8] = 3;
@@ -2628,7 +2631,7 @@ void LogicManager::HAND_Tatiana_SeekCath(HAND_PARAMS) {
if (getCharacterCurrentParams(kCharacterTatiana)[1] >= _realTime) {
if (_doneNIS[kEventTatianaTylerCompartment] || _gameTime > 2475000) {
if (getCharacterCurrentParams(kCharacterTatiana)[0]) {
- setDoor(1, 0, checkDoor(1), 10, 9);
+ setDoor(1, kCharacterCath, checkDoor(1), 10, 9);
}
_globals[kProgressFieldE4] = 0;
softReleaseAtDoor(kCharacterTatiana, 2);
@@ -2648,7 +2651,7 @@ void LogicManager::HAND_Tatiana_SeekCath(HAND_PARAMS) {
if (_doneNIS[kEventTatianaTylerCompartment] || _gameTime > 2475000) {
if (getCharacterCurrentParams(kCharacterTatiana)[0]) {
- setDoor(1, 0, checkDoor(1), 10, 9);
+ setDoor(1, kCharacterCath, checkDoor(1), 10, 9);
}
_globals[kProgressFieldE4] = 0;
softReleaseAtDoor(kCharacterTatiana, 2);
diff --git a/engines/lastexpress/debug.cpp b/engines/lastexpress/debug.cpp
index 1da22819802..56dca1b6e2a 100644
--- a/engines/lastexpress/debug.cpp
+++ b/engines/lastexpress/debug.cpp
@@ -413,7 +413,7 @@ void LogicManager::renderCharacterList(int &selectedCharacter) {
// Right panel: Character details
ImGui::BeginChild("CharacterDetails", ImVec2(0, 0), true);
- if (selectedCharacter >= 0 && selectedCharacter < 40) {
+ if (selectedCharacter < 40) {
Character *character = &getCharacter(selectedCharacter);
if (character) {
renderCharacterDetails(character, selectedCharacter);
diff --git a/engines/lastexpress/fight/fighter.cpp b/engines/lastexpress/fight/fighter.cpp
index dd86dc749c4..df774478a39 100644
--- a/engines/lastexpress/fight/fighter.cpp
+++ b/engines/lastexpress/fight/fighter.cpp
@@ -37,6 +37,10 @@ CFighter::CFighter(LastExpressEngine *engine, CFight *fight) {
_nextSequenceIdx = 0;
_hitPoints = 1;
_dodges = 0;
+
+ for (int i = 0; i < ARRAYSIZE(_seqs); i++) {
+ _seqs[i] = nullptr;
+ }
}
CFighter::~CFighter() {
@@ -61,9 +65,12 @@ void CFighter::timer() {
if (!_currentSeq) {
if (_currentSprite) {
_engine->getSpriteManager()->removeSprite(_currentSprite);
- if (!sprite || !sprite->copyScreenAndRedrawFlag)
- _engine->getSpriteManager()->queueErase(_currentSprite);
+
+ // The original did this, but at this point sprite is necessarily nullptr...
+ // if (!sprite || !sprite->copyScreenAndRedrawFlag)
+ _engine->getSpriteManager()->queueErase(_currentSprite);
}
+
_currentSprite = sprite;
return;
}
diff --git a/engines/lastexpress/lastexpress.cpp b/engines/lastexpress/lastexpress.cpp
index 48fe88822a0..ab72a5080a5 100644
--- a/engines/lastexpress/lastexpress.cpp
+++ b/engines/lastexpress/lastexpress.cpp
@@ -77,6 +77,7 @@ LastExpressEngine::~LastExpressEngine() {
SAFE_DELETE(_saveMan);
SAFE_DELETE(_clock);
SAFE_DELETE(_vcr);
+ SAFE_DELETE(_soundMutex);
//_debugger is deleted by Engine
diff --git a/engines/lastexpress/memory.cpp b/engines/lastexpress/memory.cpp
index b4389dd5e53..968d6c0988c 100644
--- a/engines/lastexpress/memory.cpp
+++ b/engines/lastexpress/memory.cpp
@@ -85,8 +85,8 @@ void MemoryManager::initMem() {
_engine->getMessageManager()->_autoMessages[i].clear();
}
- memset(_engine->getLogicManager()->_globals, 0, sizeof(128 * sizeof(int32)));
- memset(_engine->getLogicManager()->_doneNIS, 0, sizeof(512));
+ memset(_engine->getLogicManager()->_globals, 0, 128 * sizeof(int32));
+ memset(_engine->getLogicManager()->_doneNIS, 0, 512);
_engine->getGraphicsManager()->_cursorsDataHeader = (CursorHeader *)_engine->_cursorsMemoryPool;
_engine->getGraphicsManager()->_iconsBitmapData = (PixMap *)(_engine->_cursorsMemoryPool + sizeof(CursorHeader) * kCursorMAX);
diff --git a/engines/lastexpress/sound/mixer.cpp b/engines/lastexpress/sound/mixer.cpp
index afeaac99577..40817fedf37 100644
--- a/engines/lastexpress/sound/mixer.cpp
+++ b/engines/lastexpress/sound/mixer.cpp
@@ -372,12 +372,6 @@ void static decodeSamples(int32 volumeLevel, int32 stepSizeIdx, int32 initSample
// Process and write the first sample...
idx = (compressedBuffer[count] >> 4);
- // This check SHOULDN'T be needed, but I don't want to risk out of bounds accesses...
- if (idx + (stepSize >> 2) > 1424) {
- warning("invalid stepSize %d >> 2 == %d", stepSize, (stepSize >> 2));
- stepSize = 0;
- }
-
int32 sample1 = adpcmDeltaTable[idx + (stepSize >> 2)] + initialPredictor;
int32 newStepSize = adpcmStepSizeTable[idx + (stepSize >> 2)];
diff --git a/engines/lastexpress/sound/sound.cpp b/engines/lastexpress/sound/sound.cpp
index 120b033f21d..aa262396c34 100644
--- a/engines/lastexpress/sound/sound.cpp
+++ b/engines/lastexpress/sound/sound.cpp
@@ -516,6 +516,12 @@ void SoundManager::saveSoundInfo(CVCRFile *file) {
saveSlot->priority = j->_priority;
strncpy(saveSlot->name1, j->_name1, sizeof(saveSlot->name1));
strncpy(saveSlot->name2, j->_name2, sizeof(saveSlot->name2));
+
+ // I have verified that ANY possible name in here won't be longer than 12 characters,
+ // so it's safe to put this here, like Coverity asked to...
+ saveSlot->name1[15] = '\0';
+ saveSlot->name2[15] = '\0';
+
file->writeRLE(saveSlot, sizeof(SaveSlot), 1);
}
}
More information about the Scummvm-git-logs
mailing list