[Scummvm-git-logs] scummvm master -> 4e3c2ec299e4a4eb816157cd7c08edddc6222d47

[criezy]

This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
65124a3745 AGS: Fix implementation of std::list::insert
9144ec7131 AGS: Add sanity check on iterators in implementation of std::list::splice
4e3c2ec299 AGS: Invalidate iterators to MRU list in SpriteCache when the item is erased


Commit: 65124a3745a9da6fe0f982405cfe863d6cd2a377
    https://github.com/scummvm/scummvm/commit/65124a3745a9da6fe0f982405cfe863d6cd2a377
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-18T22:20:40+01:00

Commit Message:
AGS: Fix implementation of std::list::insert

Instead of returning an iterator to the inserted item, it was
returning an iterator to the next item. This was for example causing
bugs in the SpriteCache.

Changed paths:
    engines/ags/lib/std/list.h


diff --git a/engines/ags/lib/std/list.h b/engines/ags/lib/std/list.h
index 0b198e5dbea..09cef3ae29e 100644
--- a/engines/ags/lib/std/list.h
+++ b/engines/ags/lib/std/list.h
@@ -59,7 +59,7 @@ public:
 	typename Common::List<T>::iterator insert(typename Common::List<T>::iterator pos,
 			const T &element) {
 		Common::List<T>::insert(pos, element);
-		return pos;
+		return --pos;
 	}
 
 	reverse_iterator rbegin() {


Commit: 9144ec71310b6b6c635fbb22902815e7cb584191
    https://github.com/scummvm/scummvm/commit/9144ec71310b6b6c635fbb22902815e7cb584191
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-18T22:20:40+01:00

Commit Message:
AGS: Add sanity check on iterators in implementation of std::list::splice

In particular the SpriteCache class can call this function with both
iterators pointing to the same item (when trying to move an item to
the start of thr MRU list, and it is already the first item).

Changed paths:
    engines/ags/lib/std/list.h


diff --git a/engines/ags/lib/std/list.h b/engines/ags/lib/std/list.h
index 09cef3ae29e..1a4bd337d37 100644
--- a/engines/ags/lib/std/list.h
+++ b/engines/ags/lib/std/list.h
@@ -71,9 +71,9 @@ public:
 
 	void splice(typename Common::List<T>::iterator pos, list<T>& /*other*/, typename Common::List<T>::iterator it ) {
 		// We insert it before pos in this list
-		typename Common::List<T>::NodeBase *n = static_cast<typename Common::List<T>::NodeBase *>(it._node);
-		typename Common::List<T>::NodeBase *nPos = static_cast<typename Common::List<T>::NodeBase*>(pos._node);
-		if (n == nPos || n->_next == nPos)
+		typename Common::List<T>::NodeBase *n = it._node;
+		typename Common::List<T>::NodeBase *nPos = pos._node;
+		if (n == nullptr || nPos == nullptr || n == nPos || n->_next == nPos)
 			return;
 		// Remove from current position
 		n->_prev->_next = n->_next;


Commit: 4e3c2ec299e4a4eb816157cd7c08edddc6222d47
    https://github.com/scummvm/scummvm/commit/4e3c2ec299e4a4eb816157cd7c08edddc6222d47
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-18T22:20:40+01:00

Commit Message:
AGS: Invalidate iterators to MRU list in SpriteCache when the item is erased

The implementation of std::list::erase invalidates iterators to the
erased item. However our implementation in Common::List does not.
And the SpriteCache code relied on this behaviour. So instead we
have to invalidate the iterators manually.

Changed paths:
    engines/ags/shared/ac/sprite_cache.cpp


diff --git a/engines/ags/shared/ac/sprite_cache.cpp b/engines/ags/shared/ac/sprite_cache.cpp
index c05c7ed7025..404e1503741 100644
--- a/engines/ags/shared/ac/sprite_cache.cpp
+++ b/engines/ags/shared/ac/sprite_cache.cpp
@@ -236,6 +236,9 @@ void SpriteCache::DisposeOldest() {
 	if (!_spriteData[sprnum].IsAssetSprite()) {
 		Debug::Printf(kDbgGroup_SprCache, kDbgMsg_Error, "SpriteCache::DisposeOldest: in MRU list sprite %d is external or does not exist", sprnum);
 		_mru.erase(it);
+		// std::list::erase() invalidates iterators to the erased item.
+		// But our implementation does not.
+		_spriteData[sprnum].MruIt._node = nullptr;
 		return;
 	}
 	// Delete the image, unless is locked
@@ -250,6 +253,9 @@ void SpriteCache::DisposeOldest() {
 	}
 	// Remove from the mru list
 	_mru.erase(it);
+	// std::list::erase() invalidates iterators to the erased item.
+	// But our implementation does not.
+	_spriteData[sprnum].MruIt._node = nullptr;
 }
 
 void SpriteCache::DisposeAll() {
@@ -279,6 +285,9 @@ void SpriteCache::Precache(sprkey_t index) {
 		sprSize = _spriteData[index].Size;
 		// Remove locked sprite from the MRU list
 		_mru.erase(_spriteData[index].MruIt);
+		// std::list::erase() invalidates iterators to the erased item.
+		// But our implementation does not.
+		_spriteData[index].MruIt._node = nullptr;
 	}
 
 	// make sure locked sprites can't fill the cache