[Scummvm-git-logs] scummvm master -> 58c83dcd140137cf5c5808a95e7b0711fa556933

[csnover]

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:
e29f60858d SCI32: Clarify code comment about Steam GK2 RESMAP.001
58c83dcd14 COMMON: Make SpanOwner copy assignment make a copy of the owned Span


Commit: e29f60858dc4172f91d235219e76a9e13e4d72f9
    https://github.com/scummvm/scummvm/commit/e29f60858dc4172f91d235219e76a9e13e4d72f9
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-06-08T00:28:44-05:00

Commit Message:
SCI32: Clarify code comment about Steam GK2 RESMAP.001

Changed paths:
    engines/sci/resource.cpp


diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index 42c1027..a931d87 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -667,7 +667,7 @@ int ResourceManager::addAppropriateSources() {
 
 			if (!foundVolume &&
 				// GK2 on Steam comes with an extra bogus resource map file;
-				// ignore it
+				// ignore it instead of treating it as a bad resource
 				(g_sci->getGameId() != GID_GK2 || mapFiles.size() != 2 || mapNumber != 1)) {
 
 				warning("Could not find corresponding volume for %s", mapName.c_str());


Commit: 58c83dcd140137cf5c5808a95e7b0711fa556933
    https://github.com/scummvm/scummvm/commit/58c83dcd140137cf5c5808a95e7b0711fa556933
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-06-08T10:45:55-05:00

Commit Message:
COMMON: Make SpanOwner copy assignment make a copy of the owned Span

To move data from one SpanOwner to another, use `moveFrom`.
Thanks @waltervn for pointing out the problem.

Changed paths:
    common/span.h
    engines/sci/graphics/cursor.cpp
    test/common/span.h


diff --git a/common/span.h b/common/span.h
index 293000381..e0b1d13 100644
--- a/common/span.h
+++ b/common/span.h
@@ -942,20 +942,21 @@ public:
 		_span.allocateFromSpan(other._span);
 	}
 
-	/**
-	 * Transfers ownership of the Span from the other owner to this owner.
-	 * If this owner already holds another Span, the old Span will be destroyed.
-	 */
 	inline SpanOwner &operator=(SpanOwner &other) {
 		if (this == &other) {
 			return *this;
 		}
 
-		if (_span.data()) {
-			delete[] const_cast<typename RemoveConst<value_type>::type *>(_span.data());
+		delete[] const_cast<typename RemoveConst<value_type>::type *>(_span.data());
+		_span.clear();
+
+		// Allocating memory when copy-assigning from an unallocated owner
+		// will break the new owner by making it appear allocated even though
+		// it doesn't (and shouldn't) contain data
+		if (other) {
+			_span.allocateFromSpan(other._span);
 		}
-		_span = other._span;
-		other.release();
+
 		return *this;
 	}
 
@@ -964,6 +965,20 @@ public:
 	}
 
 	/**
+	 * Transfers ownership of the Span from the other owner to this owner.
+	 */
+	inline SpanOwner &moveFrom(SpanOwner &other) {
+		if (this == &other) {
+			return *this;
+		}
+
+		delete[] const_cast<typename RemoveConst<value_type>::type *>(_span.data());
+		_span = other._span;
+		other.release();
+		return *this;
+	}
+
+	/**
 	 * Releases the memory owned by this SpanOwner to the caller.
 	 */
 	inline pointer release() {
diff --git a/engines/sci/graphics/cursor.cpp b/engines/sci/graphics/cursor.cpp
index 2a86009..89bb2a2 100644
--- a/engines/sci/graphics/cursor.cpp
+++ b/engines/sci/graphics/cursor.cpp
@@ -179,7 +179,7 @@ void GfxCursor::kernelSetShape(GuiResourceId resourceId) {
 		Common::SpanOwner<SciSpan<byte> > upscaledBitmap;
 		upscaledBitmap->allocate(heightWidth * heightWidth, "upscaled cursor bitmap");
 		_screen->scale2x(*rawBitmap, *upscaledBitmap, SCI_CURSOR_SCI0_HEIGHTWIDTH, SCI_CURSOR_SCI0_HEIGHTWIDTH);
-		rawBitmap = upscaledBitmap;
+		rawBitmap.moveFrom(upscaledBitmap);
 	}
 
 	if (hotspot.x >= heightWidth || hotspot.y >= heightWidth) {
diff --git a/test/common/span.h b/test/common/span.h
index aa3ee9d..1c1a0ad 100644
--- a/test/common/span.h
+++ b/test/common/span.h
@@ -269,9 +269,26 @@ public:
 
 		{
 			Common::SpanOwner<Common::Span<byte> > owner2;
+			TS_ASSERT(owner2->data() == nullptr);
+			owner2 = owner;
+			TS_ASSERT(owner2->data() != nullptr);
+			TS_ASSERT_DIFFERS(owner->data(), owner2->data());
+
+			for (int i = 0; i < 3; ++i) {
+				TS_ASSERT_EQUALS(owner2->getUint8At(i), 'a' + i);
+				TS_ASSERT_EQUALS((*owner2)[i], 'a' + i);
+			}
+
+			TS_ASSERT_EQUALS((bool)owner2, true);
+			owner2.release();
+			TS_ASSERT_EQUALS((bool)owner2, false);
+		}
+
+		{
+			Common::SpanOwner<Common::Span<byte> > owner2;
 			TS_ASSERT_EQUALS((bool)owner, true);
 			void *dataPtr = owner->data();
-			owner2 = owner;
+			owner2.moveFrom(owner);
 			TS_ASSERT_EQUALS((bool)owner, false);
 			TS_ASSERT(owner->data() == nullptr);
 			TS_ASSERT_EQUALS(owner2->data(), dataPtr);
@@ -350,7 +367,7 @@ public:
 			Common::SpanOwner<Common::NamedSpan<byte> > owner2;
 			TS_ASSERT_EQUALS((bool)owner, true);
 			void *dataPtr = owner->data();
-			owner2 = owner;
+			owner2.moveFrom(owner);
 			TS_ASSERT_EQUALS((bool)owner, false);
 			TS_ASSERT(owner->data() == nullptr);
 			TS_ASSERT_EQUALS(owner2->data(), dataPtr);