[Scummvm-git-logs] scummvm master -> 3966bcec323439333031ef9feda96bd34fe6ac65

mduggan mgithub at guarana.org
Tue May 5 12:07:15 UTC 2020


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:
77850a75e8 ULTIMA1: Fix small coverity issues
3966bcec32 ULTIMA8: More coverity fixes


Commit: 77850a75e853f3820f0c4e582275a9b7da5bd5cd
    https://github.com/scummvm/scummvm/commit/77850a75e853f3820f0c4e582275a9b7da5bd5cd
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-05T21:06:59+09:00

Commit Message:
ULTIMA1: Fix small coverity issues

Changed paths:
    engines/ultima/ultima1/maps/map_tile.h
    engines/ultima/ultima1/u1dialogs/dialog.cpp
    engines/ultima/ultima1/widgets/bard.cpp
    engines/ultima/ultima1/widgets/dungeon_widget.cpp
    engines/ultima/ultima1/widgets/overworld_monster.cpp


diff --git a/engines/ultima/ultima1/maps/map_tile.h b/engines/ultima/ultima1/maps/map_tile.h
index 2974f31767..037586706c 100644
--- a/engines/ultima/ultima1/maps/map_tile.h
+++ b/engines/ultima/ultima1/maps/map_tile.h
@@ -53,7 +53,7 @@ public:
 	/**
 	 * Constructor
 	 */
-	U1MapTile() : Shared::Maps::MapTile(), _item(0), _locationNum(-1) {}
+	U1MapTile() : Shared::Maps::MapTile(), _item(0), _locationNum(-1), _map(nullptr) {}
 
 	/**
 	 * Set the active map
diff --git a/engines/ultima/ultima1/u1dialogs/dialog.cpp b/engines/ultima/ultima1/u1dialogs/dialog.cpp
index 4c2e43de53..b051f3c760 100644
--- a/engines/ultima/ultima1/u1dialogs/dialog.cpp
+++ b/engines/ultima/ultima1/u1dialogs/dialog.cpp
@@ -60,6 +60,7 @@ void Dialog::getInput(bool isNumeric, size_t maxCharacters) {
 void Dialog::draw() {
 	// Redraw the game's info area
 	U1Gfx::Info *infoArea = dynamic_cast<U1Gfx::Info *>(_game->findByName("Info"));
+	assert(infoArea);
 	infoArea->draw();
 }
 
diff --git a/engines/ultima/ultima1/widgets/bard.cpp b/engines/ultima/ultima1/widgets/bard.cpp
index 65aad5cdfa..ea6a6ebec0 100644
--- a/engines/ultima/ultima1/widgets/bard.cpp
+++ b/engines/ultima/ultima1/widgets/bard.cpp
@@ -82,6 +82,7 @@ bool Bard::subtractHitPoints(uint amount) {
 	bool result = Person::subtractHitPoints(amount);
 	if (result) {
 		Maps::MapCastle *map = dynamic_cast<Maps::MapCastle *>(_map);
+		assert(map);
 		addInfoMsg(_game->_res->FOUND_KEY);
 		map->_castleKey = 1;
 	}
diff --git a/engines/ultima/ultima1/widgets/dungeon_widget.cpp b/engines/ultima/ultima1/widgets/dungeon_widget.cpp
index 66d0ac6521..6c39a6ad3f 100644
--- a/engines/ultima/ultima1/widgets/dungeon_widget.cpp
+++ b/engines/ultima/ultima1/widgets/dungeon_widget.cpp
@@ -34,7 +34,7 @@ DungeonWidget::DungeonWidget(Ultima1Game *game, Maps::MapBase *map, DungeonWidge
 		const Point &pt) : Shared::Maps::DungeonWidget(game, map, pt), _widgetId(widgetId) {
 }
 
-DungeonWidget::DungeonWidget(Ultima1Game *game, Maps::MapBase *map) : Shared::Maps::DungeonWidget(game, map) {
+DungeonWidget::DungeonWidget(Ultima1Game *game, Maps::MapBase *map) : Shared::Maps::DungeonWidget(game, map), _widgetId(MONSTER_NONE) {
 }
 
 Ultima1Game *DungeonWidget::getGame() const {
diff --git a/engines/ultima/ultima1/widgets/overworld_monster.cpp b/engines/ultima/ultima1/widgets/overworld_monster.cpp
index dd8ae2b0b7..a51c16e6e0 100644
--- a/engines/ultima/ultima1/widgets/overworld_monster.cpp
+++ b/engines/ultima/ultima1/widgets/overworld_monster.cpp
@@ -64,6 +64,7 @@ void OverworldMonster::movement() {
 
 void OverworldMonster::attackParty() {
 	Ultima1Game *game = dynamic_cast<Ultima1Game *>(_game);
+	assert(game);
 	Point playerPos = _map->_playerWidget->_position;
 	Point diff = playerPos - _position;
 	Point delta(SGN(diff.x), SGN(diff.y));


Commit: 3966bcec323439333031ef9feda96bd34fe6ac65
    https://github.com/scummvm/scummvm/commit/3966bcec323439333031ef9feda96bd34fe6ac65
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-05T21:06:59+09:00

Commit Message:
ULTIMA8: More coverity fixes

Changed paths:
    engines/ultima/ultima8/audio/remorse_music_process.cpp
    engines/ultima/ultima8/usecode/uc_list.cpp
    engines/ultima/ultima8/usecode/uc_list.h
    engines/ultima/ultima8/usecode/uc_machine.cpp


diff --git a/engines/ultima/ultima8/audio/remorse_music_process.cpp b/engines/ultima/ultima8/audio/remorse_music_process.cpp
index 25a96d888c..7eff13713e 100644
--- a/engines/ultima/ultima8/audio/remorse_music_process.cpp
+++ b/engines/ultima/ultima8/audio/remorse_music_process.cpp
@@ -61,7 +61,7 @@ static const char *TRACK_FILE_NAMES[] = {
 // p_dynamic_cast stuff
 DEFINE_RUNTIME_CLASSTYPE_CODE(RemorseMusicProcess, MusicProcess)
 
-RemorseMusicProcess::RemorseMusicProcess() : MusicProcess(), _currentTrack(0), _savedTrack(0), _playingStream(nullptr) {
+RemorseMusicProcess::RemorseMusicProcess() : MusicProcess(), _currentTrack(0), _savedTrack(0), _playingStream(nullptr), _combatMusicActive(false) {
 }
 
 RemorseMusicProcess::~RemorseMusicProcess() {
diff --git a/engines/ultima/ultima8/usecode/uc_list.cpp b/engines/ultima/ultima8/usecode/uc_list.cpp
index 6f08a83032..a199d7a6a4 100644
--- a/engines/ultima/ultima8/usecode/uc_list.cpp
+++ b/engines/ultima/ultima8/usecode/uc_list.cpp
@@ -72,7 +72,7 @@ void UCList::unionStringList(UCList &l) {
 	l.free(); // NB: do _not_ free the strings in l, since they're in this one
 }
 
-void UCList::substractStringList(const UCList &l) {
+void UCList::subtractStringList(const UCList &l) {
 	for (unsigned int i = 0; i < l._size; i++)
 		removeString(l.getStringIndex(i));
 }
diff --git a/engines/ultima/ultima8/usecode/uc_list.h b/engines/ultima/ultima8/usecode/uc_list.h
index 7c7e45078c..fbba166f89 100644
--- a/engines/ultima/ultima8/usecode/uc_list.h
+++ b/engines/ultima/ultima8/usecode/uc_list.h
@@ -34,10 +34,10 @@ namespace Ultima8 {
 // created list is a stringlist or not
 // the opcodes which do need a distinction have a operand for this.
 
-// Question: how are unionList/substractList supposed to know what to do?
+// Question: how are unionList/subtractList supposed to know what to do?
 // their behaviour differs if this is a stringlist
 
-// Question: does substractList remove _all_ occurences of elements or only 1?
+// Question: does subtractList remove _all_ occurences of elements or only 1?
 
 class UCList {
 	Std::vector<uint8> _elements;
@@ -118,7 +118,7 @@ public:
 			if (!inList(l[i]))
 				append(l[i]);
 	}
-	void substractList(const UCList &l) {
+	void subtractList(const UCList &l) {
 		for (unsigned int i = 0; i < l._size; i++)
 			remove(l[i]);
 	}
@@ -148,7 +148,7 @@ public:
 	void freeStrings();
 	void copyStringList(const UCList &l) ;
 	void unionStringList(UCList &l);
-	void substractStringList(const UCList &l);
+	void subtractStringList(const UCList &l);
 	bool stringInList(uint16 str) const;
 	void assignString(uint32 index, uint16 str);
 	void removeString(uint16 str, bool nodel = false);
diff --git a/engines/ultima/ultima8/usecode/uc_machine.cpp b/engines/ultima/ultima8/usecode/uc_machine.cpp
index a1bfe613d8..b17bbcce21 100644
--- a/engines/ultima/ultima8/usecode/uc_machine.cpp
+++ b/engines/ultima/ultima8/usecode/uc_machine.cpp
@@ -504,7 +504,7 @@ void UCMachine::execProcess(UCProcess *p) {
 		}
 		break;
 
-		case 0x19:
+		case 0x19: {
 			// 19 02
 			// add two stringlists, removing duplicates
 			ui32a = cs.readByte();
@@ -515,28 +515,42 @@ void UCMachine::execProcess(UCProcess *p) {
 			}
 			ui16a = p->_stack.pop2();
 			ui16b = p->_stack.pop2();
-			getList(ui16b)->unionStringList(*getList(ui16a));
-			freeStringList(ui16a); // contents are actually freed in unionSL
+			UCList *srclist = getList(ui16a);
+			UCList *dstlist = getList(ui16b);
+			if (!srclist || !dstlist) {
+				perr << "Invalid list param to union slist" << Std::endl;
+				error = true;
+			} else {
+				dstlist->unionStringList(*srclist);
+				freeStringList(ui16a); // contents are actually freed in unionSL
+			}
 			p->_stack.push2(ui16b);
 			LOGPF(("union slist\t(%02X)\n", ui32a));
 			break;
-
-		case 0x1A:
+		}
+		case 0x1A: {
 			// 1A
-			// substract string list
+			// subtract string list
 			// NB: this one takes a length parameter in crusader. (not in U8)!!
 			// (or rather, it seems it takes one after all? -wjp,20030511)
 			ui32a = cs.readByte(); // elementsize
 			ui32a = 2;
 			ui16a = p->_stack.pop2();
 			ui16b = p->_stack.pop2();
-			getList(ui16b)->substractStringList(*getList(ui16a));
-			freeStringList(ui16a);
+			UCList *srclist = getList(ui16a);
+			UCList *dstlist = getList(ui16b);
+			if (!srclist || !dstlist) {
+				perr << "Invalid list param to subtract slist" << Std::endl;
+				error = true;
+			} else {
+				dstlist->subtractStringList(*srclist);
+				freeStringList(ui16a);
+			}
 			p->_stack.push2(ui16b);
 			LOGPF(("remove slist\t(%02X)\n", ui32a));
 			break;
-
-		case 0x1B:
+		}
+		case 0x1B: {
 			// 1B xx
 			// pop two lists from the stack and remove the 2nd from the 1st
 			// (free the originals? order?)
@@ -544,12 +558,19 @@ void UCMachine::execProcess(UCProcess *p) {
 			ui32a = cs.readByte(); // elementsize
 			ui16a = p->_stack.pop2();
 			ui16b = p->_stack.pop2();
-			getList(ui16b)->substractList(*getList(ui16a));
-			freeList(ui16a);
+			UCList *srclist = getList(ui16a);
+			UCList *dstlist = getList(ui16b);
+			if (!srclist || !dstlist) {
+				perr << "Invalid list param to remove from slist" << Std::endl;
+				error = true;
+			} else {
+				dstlist->subtractList(*srclist);
+				freeList(ui16a);
+			}
 			p->_stack.push2(ui16b);
 			LOGPF(("remove list\t(%02X)\n", ui32a));
 			break;
-
+		}
 		case 0x1C:
 			// 1C
 			// subtract two 16 bit integers
@@ -1553,6 +1574,7 @@ void UCMachine::execProcess(UCProcess *p) {
 					perr << "Warning: invalid src list passed to slist copy"
 						 << Std::endl;
 					ui16b = 0;
+					delete l;
 					break;
 				}
 				l->copyStringList(*srclist);
@@ -1594,7 +1616,7 @@ void UCMachine::execProcess(UCProcess *p) {
 
 		case 0x6E:
 			// 6E xx
-			// substract xx from stack pointer
+			// subtract xx from stack pointer
 			// (effect on SP is the same as popping xx bytes)
 			si8a = static_cast<int8>(cs.readByte());
 			p->_stack.addSP(-si8a);




More information about the Scummvm-git-logs mailing list