From 58c83dcd140137cf5c5808a95e7b0711fa556933 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Wed, 24 May 2017 23:12:23 -0500
Subject: 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.
---
 common/span.h                   | 31 +++++++++++++++++++++++--------
 engines/sci/graphics/cursor.cpp |  2 +-
 test/common/span.h              | 21 +++++++++++++++++++--
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/common/span.h b/common/span.h
index 2930003811..e0b1d139ca 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;
 	}
 
@@ -963,6 +964,20 @@ public:
 		delete[] const_cast<typename RemoveConst<value_type>::type *>(_span.data());
 	}
 
+	/**
+	 * 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.
 	 */
diff --git a/engines/sci/graphics/cursor.cpp b/engines/sci/graphics/cursor.cpp
index 2a8600969e..89bb2a2307 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 aa3ee9d22f..1c1a0adcda 100644
--- a/test/common/span.h
+++ b/test/common/span.h
@@ -267,11 +267,28 @@ public:
 			TS_ASSERT_EQUALS((bool)owner2, false);
 		}
 
+		{
+			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);
-- 
cgit v1.2.3