[Scummvm-git-logs] scummvm master -> 83eefd4214638df5b99c301d05074a3e07d85b3a

criezy noreply at scummvm.org
Wed Nov 27 21:23:42 UTC 2024


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

Summary:
83eefd4214 AGS: Engine: create ccInstances wrapped in std::unique_ptr


Commit: 83eefd4214638df5b99c301d05074a3e07d85b3a
    https://github.com/scummvm/scummvm/commit/83eefd4214638df5b99c301d05074a3e07d85b3a
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2024-11-27T21:23:33Z

Commit Message:
AGS: Engine: create ccInstances wrapped in std::unique_ptr

This also fixed a potential memory leak in ccInstance::CreateEx().

>From upstream 30167df0690e0a771a8eb4f14a4d6a72da526b49

Also remove unreachable code (same condition checked twice in a
function) as that caused a compilation error with the changes above
otherwise. This change is part of upstream commit
881d31966cf7600c9b5628fff4c194d628196f4a

Changed paths:
    engines/ags/engine/ac/room.cpp
    engines/ags/engine/script/cc_instance.cpp
    engines/ags/engine/script/cc_instance.h
    engines/ags/engine/script/script.cpp


diff --git a/engines/ags/engine/ac/room.cpp b/engines/ags/engine/ac/room.cpp
index eadafc66ff9..c5a85f05def 100644
--- a/engines/ags/engine/ac/room.cpp
+++ b/engines/ags/engine/ac/room.cpp
@@ -958,7 +958,7 @@ void check_new_room() {
 void compile_room_script() {
 	cc_clear_error();
 
-	_G(roominst).reset(ccInstance::CreateFromScript(_GP(thisroom).CompiledScript));
+	_G(roominst) = ccInstance::CreateFromScript(_GP(thisroom).CompiledScript);
 	if (cc_has_error() || (_G(roominst) == nullptr)) {
 		quitprintf("Unable to create local script:\n%s", cc_get_error().ErrorString.GetCStr());
 	}
@@ -969,7 +969,7 @@ void compile_room_script() {
 	if (!_G(roominst)->ResolveImportFixups(_G(roominst)->instanceof.get()))
 		quitprintf("Unable to resolve import fixups in room script:\n%s", cc_get_error().ErrorString.GetCStr());
 
-	_G(roominstFork).reset(_G(roominst)->Fork());
+	_G(roominstFork) = _G(roominst)->Fork();
 	if (_G(roominstFork) == nullptr)
 		quitprintf("Unable to create forked room instance:\n%s", cc_get_error().ErrorString.GetCStr());
 
diff --git a/engines/ags/engine/script/cc_instance.cpp b/engines/ags/engine/script/cc_instance.cpp
index b23e13854cb..34e3734e9c0 100644
--- a/engines/ags/engine/script/cc_instance.cpp
+++ b/engines/ags/engine/script/cc_instance.cpp
@@ -211,13 +211,13 @@ void ccInstance::FreeInstanceStack() {
 	_GP(InstThreads).clear();
 }
 
-ccInstance *ccInstance::CreateFromScript(PScript scri) {
+std::unique_ptr<ccInstance> ccInstance::CreateFromScript(PScript scri) {
 	return CreateEx(scri, nullptr);
 }
 
-ccInstance *ccInstance::CreateEx(PScript scri, const ccInstance *joined) {
+std::unique_ptr<ccInstance> ccInstance::CreateEx(PScript scri, const ccInstance *joined) {
 	// allocate and copy all the memory with data, code and strings across
-	ccInstance *cinst = new ccInstance();
+	std::unique_ptr<ccInstance> cinst(new ccInstance());
 	if (!cinst->_Create(scri, joined)) {
 		return nullptr;
 	}
@@ -263,7 +263,7 @@ ccInstance::~ccInstance() {
 	Free();
 }
 
-ccInstance *ccInstance::Fork() {
+std::unique_ptr<ccInstance> ccInstance::Fork() {
 	return CreateEx(instanceof, this);
 }
 
diff --git a/engines/ags/engine/script/cc_instance.h b/engines/ags/engine/script/cc_instance.h
index ca5a235c94d..a736cdd42ee 100644
--- a/engines/ags/engine/script/cc_instance.h
+++ b/engines/ags/engine/script/cc_instance.h
@@ -162,14 +162,14 @@ public:
 	// when destroying all script instances, e.g. on game quit.
 	static void FreeInstanceStack();
 	// create a runnable instance of the supplied script
-	static ccInstance *CreateFromScript(PScript script);
-	static ccInstance *CreateEx(PScript scri, const ccInstance *joined);
+	static std::unique_ptr<ccInstance> CreateFromScript(PScript script);
+	static std::unique_ptr<ccInstance> CreateEx(PScript scri, const ccInstance *joined);
 	static void SetExecTimeout(unsigned sys_poll_ms, unsigned abort_ms, unsigned abort_loops);
 
 	ccInstance();
 	~ccInstance();
 	// Create a runnable instance of the same script, sharing global memory
-	ccInstance *Fork();
+	std::unique_ptr<ccInstance> Fork();
 	// Specifies that when the current function returns to the script, it
 	// will stop and return from CallInstance
 	void    Abort();
diff --git a/engines/ags/engine/script/script.cpp b/engines/ags/engine/script/script.cpp
index e3ea592c6f6..dc8e9f672d8 100644
--- a/engines/ags/engine/script/script.cpp
+++ b/engines/ags/engine/script/script.cpp
@@ -202,23 +202,22 @@ int create_global_script() {
 
 	std::vector<ccInstance *> all_insts; // gather all to resolve exports below
 	for (size_t i = 0; i < _G(numScriptModules); ++i) {
-		auto inst = ccInstance::CreateFromScript(_GP(scriptModules)[i]);
-		if (!inst)
+		_GP(moduleInst)[i] = ccInstance::CreateFromScript(_GP(scriptModules)[i]);
+		if (!_GP(moduleInst)[i])
 			return kscript_create_error;
-		_GP(moduleInst)[i].reset(inst);
-		all_insts.push_back(inst);
+		all_insts.push_back(_GP(moduleInst)[i].get()); // this is only for temp reference
 	}
 
-	_G(gameinst).reset(ccInstance::CreateFromScript(_GP(gamescript)));
+	_G(gameinst) = ccInstance::CreateFromScript(_GP(gamescript));
 	if (!_G(gameinst))
 		return kscript_create_error;
-	all_insts.push_back(_G(gameinst).get());
+	all_insts.push_back(_G(gameinst).get()); // this is only for temp reference
 
 	if (_GP(dialogScriptsScript)) {
-		_G(dialogScriptsInst).reset(ccInstance::CreateFromScript(_GP(dialogScriptsScript)));
+		_G(dialogScriptsInst) = ccInstance::CreateFromScript(_GP(dialogScriptsScript));
 		if (!_G(dialogScriptsInst))
 			return kscript_create_error;
-		all_insts.push_back(_G(dialogScriptsInst).get());
+		all_insts.push_back(_G(dialogScriptsInst).get()); // this is only for temp reference
 	}
 
 	// Resolve the script imports after all the scripts have been loaded
@@ -236,11 +235,11 @@ int create_global_script() {
 		if (!fork)
 			return kscript_create_error;
 
-		_GP(moduleInstFork)[module_idx].reset(fork);
+		_GP(moduleInstFork)[module_idx] = std::move(fork);
 		_GP(moduleRepExecAddr)[module_idx] = _GP(moduleInst)[module_idx]->GetSymbolAddress(REP_EXEC_NAME);
 	}
 
-	_G(gameinstFork).reset(_G(gameinst)->Fork());
+	_G(gameinstFork) = _G(gameinst)->Fork();
 	if (_G(gameinstFork) == nullptr)
 		return kscript_create_error;
 
@@ -318,17 +317,7 @@ static int PrepareTextScript(ccInstance *sci, const char **tsname) {
 		return -3;
 	}
 	ExecutingScript exscript;
-	// CHECKME: this conditional block will never run, because
-	// function would have quit earlier (deprecated functionality?)
-	if (sci->IsBeingRun()) {
-		auto fork = sci->Fork();
-		if (!fork)
-			quit("unable to fork instance for secondary script");
-		exscript.forkedInst.reset(fork);
-		exscript.inst = fork;
-	} else {
-		exscript.inst = sci;
-	}
+	exscript.inst = sci;
 	_G(scripts)[_G(num_scripts)] = std::move(exscript);
 	_G(curscript) = &_G(scripts)[_G(num_scripts)];
 	_G(num_scripts)++;




More information about the Scummvm-git-logs mailing list