From cad2be46ac4aea1cfe509192dd55be83f411752e Mon Sep 17 00:00:00 2001
From: Christoph Oelckers <c.oelckers@zdoom.fake>
Date: Fri, 23 Sep 2016 08:49:30 +0200
Subject: [PATCH] - fixed several Destroy methods which blanketly assumed that
 the object's pointers were valid to use without checks.

This is not the case if deserialization prematurely aborts. The entire object may be invalid if something in the deserializer I_Error's out.
---
 src/g_shared/a_decals.cpp       |  9 ++++---
 src/g_shared/a_sectoraction.cpp | 33 ++++++++++++++-----------
 src/g_shared/a_skies.cpp        |  5 +++-
 src/r_data/r_interpolate.cpp    | 44 ++++++++++++++++++++++-----------
 4 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/src/g_shared/a_decals.cpp b/src/g_shared/a_decals.cpp
index ebbaaf197..7eea30fef 100644
--- a/src/g_shared/a_decals.cpp
+++ b/src/g_shared/a_decals.cpp
@@ -116,13 +116,16 @@ void DBaseDecal::Destroy ()
 
 void DBaseDecal::Remove ()
 {
-	if (WallPrev == nullptr) Side->AttachedDecals = WallNext;
+	if (WallPrev == nullptr)
+	{
+		if (Side != nullptr) Side->AttachedDecals = WallNext;
+	}
 	else WallPrev->WallNext = WallNext;
 
 	if (WallNext != nullptr) WallNext->WallPrev = WallPrev;
 
-	WallPrev = NULL;
-	WallNext = NULL;
+	WallPrev = nullptr;
+	WallNext = nullptr;
 }
 
 void DBaseDecal::Serialize(FSerializer &arc)
diff --git a/src/g_shared/a_sectoraction.cpp b/src/g_shared/a_sectoraction.cpp
index 397fa1a89..da3830d74 100644
--- a/src/g_shared/a_sectoraction.cpp
+++ b/src/g_shared/a_sectoraction.cpp
@@ -49,23 +49,26 @@ bool ASectorAction::IsActivatedByUse() const
 
 void ASectorAction::Destroy ()
 {
-	// Remove ourself from this sector's list of actions
-	AActor *probe = Sector->SecActTarget;
-	union
+	if (Sector != nullptr)
 	{
-		AActor **act;
-		ASectorAction **secact;
-	} prev;
-	prev.secact = &Sector->SecActTarget;
+		// Remove ourself from this sector's list of actions
+		AActor *probe = Sector->SecActTarget;
+		union
+		{
+			AActor **act;
+			ASectorAction **secact;
+		} prev;
+		prev.secact = &Sector->SecActTarget;
 
-	while (probe && probe != this)
-	{
-		prev.act = &probe->tracer;
-		probe = probe->tracer;
-	}
-	if (probe != NULL)
-	{
-		*prev.act = probe->tracer;
+		while (probe && probe != this)
+		{
+			prev.act = &probe->tracer;
+			probe = probe->tracer;
+		}
+		if (probe != nullptr)
+		{
+			*prev.act = probe->tracer;
+		}
 	}
 
 	Super::Destroy ();
diff --git a/src/g_shared/a_skies.cpp b/src/g_shared/a_skies.cpp
index 72091e098..89c785e3e 100644
--- a/src/g_shared/a_skies.cpp
+++ b/src/g_shared/a_skies.cpp
@@ -170,7 +170,10 @@ void ASectorSilencer::BeginPlay ()
 
 void ASectorSilencer::Destroy ()
 {
-	Sector->Flags &= ~SECF_SILENT;
+	if (Sector != nullptr)
+	{
+		Sector->Flags &= ~SECF_SILENT;
+	}
 	Super::Destroy ();
 }
 
diff --git a/src/r_data/r_interpolate.cpp b/src/r_data/r_interpolate.cpp
index df8773879..c944f0fa5 100644
--- a/src/r_data/r_interpolate.cpp
+++ b/src/r_data/r_interpolate.cpp
@@ -418,15 +418,18 @@ DSectorPlaneInterpolation::DSectorPlaneInterpolation(sector_t *_sector, bool _pl
 
 void DSectorPlaneInterpolation::Destroy()
 {
-	if (ceiling) 
+	if (sector != nullptr)
 	{
-		sector->interpolations[sector_t::CeilingMove] = NULL;
+		if (ceiling)
+		{
+			sector->interpolations[sector_t::CeilingMove] = nullptr;
+		}
+		else
+		{
+			sector->interpolations[sector_t::FloorMove] = nullptr;
+		}
+		sector = nullptr;
 	}
-	else 
-	{
-		sector->interpolations[sector_t::FloorMove] = NULL;
-	}
-
 	for(unsigned i=0; i<attached.Size(); i++)
 	{
 		attached[i]->DelRef();
@@ -593,13 +596,17 @@ DSectorScrollInterpolation::DSectorScrollInterpolation(sector_t *_sector, bool _
 
 void DSectorScrollInterpolation::Destroy()
 {
-	if (ceiling) 
+	if (sector != nullptr)
 	{
-		sector->interpolations[sector_t::CeilingScroll] = NULL;
-	}
-	else 
-	{
-		sector->interpolations[sector_t::FloorScroll] = NULL;
+		if (ceiling)
+		{
+			sector->interpolations[sector_t::CeilingScroll] = nullptr;
+		}
+		else
+		{
+			sector->interpolations[sector_t::FloorScroll] = nullptr;
+		}
+		sector = nullptr;
 	}
 	Super::Destroy();
 }
@@ -694,7 +701,11 @@ DWallScrollInterpolation::DWallScrollInterpolation(side_t *_side, int _part)
 
 void DWallScrollInterpolation::Destroy()
 {
-	side->textures[part].interpolation = NULL;
+	if (side != nullptr)
+	{
+		side->textures[part].interpolation = nullptr;
+		side = nullptr;
+	}
 	Super::Destroy();
 }
 
@@ -788,7 +799,10 @@ DPolyobjInterpolation::DPolyobjInterpolation(FPolyObj *po)
 
 void DPolyobjInterpolation::Destroy()
 {
-	poly->interpolation = NULL;
+	if (poly != nullptr)
+	{
+		poly->interpolation = nullptr;
+	}
 	Super::Destroy();
 }