Move most of the implementation details out of os_forbid.h, fixes crashing if a module adding a forbid is unloaded without removing the forbid

3 files changed, 60 insertions, 46 deletions

diff --git a/include/modules/os_forbid.h b/include/modules/os_forbid.h
index 3f1fe9a7f..fe96ba5c6 100644
--- a/include/modules/os_forbid.h
+++ b/include/modules/os_forbid.h
@@ -10,7 +10,7 @@ enum ForbidType
 	FT_SIZE
 };
 
-struct ForbidData : Serializable
+struct ForbidData
 {
 	Anope::string mask;
 	Anope::string creator;
@@ -19,9 +19,9 @@ struct ForbidData : Serializable
 	time_t expires;
 	ForbidType type;
 
-	ForbidData() : Serializable("ForbidData") { }
-	void Serialize(Serialize::Data &data) const anope_override;
-	static Serializable* Unserialize(Serializable *obj, Serialize::Data &data);
+	virtual ~ForbidData() { }
+ protected:
+	ForbidData() : created(0), expires(0) { }
 };
 
 class ForbidService : public Service
@@ -33,6 +33,8 @@ class ForbidService : public Service
 
 	virtual void RemoveForbid(ForbidData *d) = 0;
 
+	virtual ForbidData* CreateForbid() = 0;
+
 	virtual ForbidData *FindForbid(const Anope::string &mask, ForbidType type) = 0;
 
 	virtual std::vector<ForbidData *> GetForbids() = 0;
@@ -40,43 +42,5 @@ class ForbidService : public Service
 
 static ServiceReference<ForbidService> forbid_service("ForbidService", "forbid");
 
-void ForbidData::Serialize(Serialize::Data &data) const
-{
-	data["mask"] << this->mask;
-	data["creator"] << this->creator;
-	data["reason"] << this->reason;
-	data["created"] << this->created;
-	data["expires"] << this->expires;
-	data["type"] << this->type;
-}
-
-Serializable* ForbidData::Unserialize(Serializable *obj, Serialize::Data &data)
-{
-	if (!forbid_service)
-		return NULL;
-
-	ForbidData *fb;
-	if (obj)
-		fb = anope_dynamic_static_cast<ForbidData *>(obj);
-	else
-		fb = new ForbidData;
-
-	data["mask"] >> fb->mask;
-	data["creator"] >> fb->creator;
-	data["reason"] >> fb->reason;
-	data["created"] >> fb->created;
-	data["expires"] >> fb->expires;
-	unsigned int t;
-	data["type"] >> t;
-	fb->type = static_cast<ForbidType>(t);
-
-	if (t > FT_SIZE - 1)
-		return NULL;
-
-	if (!obj)
-		forbid_service->AddForbid(fb);
-	return fb;
-}
-
 #endif
 
diff --git a/modules/commands/os_forbid.cpp b/modules/commands/os_forbid.cpp
index c1fe1af8b..11215ab05 100644
--- a/modules/commands/os_forbid.cpp
+++ b/modules/commands/os_forbid.cpp
@@ -14,6 +14,51 @@
 
 static ServiceReference<NickServService> nickserv("NickServService", "NickServ");
 
+struct ForbidDataImpl : ForbidData, Serializable
+{
+	ForbidDataImpl() : Serializable("ForbidData") { }
+	void Serialize(Serialize::Data &data) const anope_override;
+	static Serializable* Unserialize(Serializable *obj, Serialize::Data &data);
+};
+
+void ForbidDataImpl::Serialize(Serialize::Data &data) const
+{
+	data["mask"] << this->mask;
+	data["creator"] << this->creator;
+	data["reason"] << this->reason;
+	data["created"] << this->created;
+	data["expires"] << this->expires;
+	data["type"] << this->type;
+}
+
+Serializable* ForbidDataImpl::Unserialize(Serializable *obj, Serialize::Data &data)
+{
+	if (!forbid_service)
+		return NULL;
+
+	ForbidDataImpl *fb;
+	if (obj)
+		fb = anope_dynamic_static_cast<ForbidDataImpl *>(obj);
+	else
+		fb = new ForbidDataImpl();
+
+	data["mask"] >> fb->mask;
+	data["creator"] >> fb->creator;
+	data["reason"] >> fb->reason;
+	data["created"] >> fb->created;
+	data["expires"] >> fb->expires;
+	unsigned int t;
+	data["type"] >> t;
+	fb->type = static_cast<ForbidType>(t);
+
+	if (t > FT_SIZE - 1)
+		return NULL;
+
+	if (!obj)
+		forbid_service->AddForbid(fb);
+	return fb;
+}
+
 class MyForbidService : public ForbidService
 {
 	Serialize::Checker<std::vector<ForbidData *>[FT_SIZE - 1]> forbid_data;
@@ -43,6 +88,11 @@ class MyForbidService : public ForbidService
 		delete d;
 	}
 
+	ForbidData *CreateForbid() anope_override
+	{
+		return new ForbidDataImpl();
+	}
+
 	ForbidData *FindForbid(const Anope::string &mask, ForbidType ftype) anope_override
 	{
 		for (unsigned i = this->forbids(ftype).size(); i > 0; --i)
@@ -157,7 +207,7 @@ class CommandOSForbid : public Command
 			bool created = false;
 			if (d == NULL)
 			{
-				d = new ForbidData();
+				d = new ForbidDataImpl();
 				created = true;
 			}
 
@@ -379,7 +429,7 @@ class OSForbid : public Module
 
  public:
 	OSForbid(const Anope::string &modname, const Anope::string &creator) : Module(modname, creator, VENDOR),
-		forbidService(this), forbiddata_type("ForbidData", ForbidData::Unserialize), commandosforbid(this)
+		forbidService(this), forbiddata_type("ForbidData", ForbidDataImpl::Unserialize), commandosforbid(this)
 	{
 
 	}
diff --git a/modules/database/db_old.cpp b/modules/database/db_old.cpp
index 8c322d02e..ddc488084 100644
--- a/modules/database/db_old.cpp
+++ b/modules/database/db_old.cpp
@@ -622,7 +622,7 @@ static void LoadNicks()
 				if (!forbid)
 					continue;
 
-				ForbidData *d = new ForbidData();
+				ForbidData *d = forbid->CreateForbid();
 				d->mask = nc->display;
 				d->creator = last_usermask;
 				d->reason = last_realname;
@@ -1031,7 +1031,7 @@ static void LoadChannels()
 				if (!forbid)
 					continue;
 
-				ForbidData *d = new ForbidData();
+				ForbidData *d = forbid->CreateForbid();
 				d->mask = ci->name;
 				d->creator = forbidby;
 				d->reason = forbidreason;