[Scummvm-git-logs] scummvm master -> d9bbf9775cda4442e15b038a5573d5af3c6696d9

[dreammaster]

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

Summary:
0416016b3d AGS: Add note from ivan-mogilko about MoveCharacterBlocking return values
4ae9574c64 AGS: Simplified use of FSLocation and ResolvedPath
0ba468d1c9 AGS: CreateAllDirectories works explicitly with sub_dirs parameter
215139e49d AGS: Add fast-path for file lookup
8f3b1e4b0e AGS: print all missing script imports for current script module
ddb5daca4c AGS: improve debug level and cc_error output for missing imports
7238f9df51 AGS: Fix memory leak in ci_find_file error paths
284437b22e AGS: In ci_find_file, fix directory handle leak on error
fabdbb1e20 AGS: In ci_find_file fix omission to check lstat return value
bfd25ba440 AGS: Reorganized ReadExtData functions into a DataExtReader class
ddf30d3234 AGS: implement GetOldBlockName() in RoomBlockReader, TRABlockReader
7b42f9c350 AGS: Optional block over-read leeway, for TRABlockReader
f19dd1c506 AGS: few cast fixes
f5a019d3b4 AGS: corrected GetDisplayDepthForNativeDepth
f6e5133edd AGS: In ci_find_file remove gratuitous chdir() code
d9bbf9775c AGS: Rewrote ci_find_file() into File::FindFileCI()


Commit: 0416016b3daf911fd2e418c90c1a727f7c574912
    https://github.com/scummvm/scummvm/commit/0416016b3daf911fd2e418c90c1a727f7c574912
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:56-07:00

Commit Message:
AGS: Add note from ivan-mogilko about MoveCharacterBlocking return values

Changed paths:
    engines/ags/engine/ac/global_character.cpp


diff --git a/engines/ags/engine/ac/global_character.cpp b/engines/ags/engine/ac/global_character.cpp
index 2c1288764c..77d08ef42c 100644
--- a/engines/ags/engine/ac/global_character.cpp
+++ b/engines/ags/engine/ac/global_character.cpp
@@ -367,6 +367,11 @@ int MoveCharacterBlocking(int chaa, int xx, int yy, int direct) {
 		MoveCharacter(chaa, xx, yy);
 
 	GameLoopUntilNotMoving(&_GP(game).chars[chaa].walking);
+
+	// TODO: See https://github.com/adventuregamestudio/ags/issues/1331#issuecomment-877524051
+	// To summarize, looks like there was a whole system of returned values, which was later
+	// just scrapped, so the ways to restore it is either extensive testing of all these
+	// functions using pre-2.72 editor/engine, or reverse engineer the code maybe.
 	return 1;
 }
 


Commit: 4ae9574c6423f047665bf2b9e5ece914f527887d
    https://github.com/scummvm/scummvm/commit/4ae9574c6423f047665bf2b9e5ece914f527887d
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:56-07:00

Commit Message:
AGS: Simplified use of FSLocation and ResolvedPath

>From upstream 89f62716b7f200250326b041d9d58e65dde896e1

Changed paths:
    engines/ags/engine/ac/file.cpp
    engines/ags/engine/ac/game.cpp
    engines/ags/engine/ac/path_helper.h


diff --git a/engines/ags/engine/ac/file.cpp b/engines/ags/engine/ac/file.cpp
index ac918757b8..eeb93a422a 100644
--- a/engines/ags/engine/ac/file.cpp
+++ b/engines/ags/engine/ac/file.cpp
@@ -236,19 +236,19 @@ String PreparePathForWriting(const FSLocation &fsloc, const String &filename) {
 FSLocation GetGlobalUserConfigDir() {
 	String dir = _G(platform)->GetUserGlobalConfigDirectory();
 	if (Path::IsRelativePath(dir)) // relative dir is resolved relative to the game data dir
-		return FSLocation(_GP(ResPaths).DataDir, Path::ConcatPaths(_GP(ResPaths).DataDir, dir));
-	return FSLocation(dir, dir);
+		return FSLocation(_GP(ResPaths).DataDir, dir);
+	return FSLocation(dir);
 }
 
 FSLocation GetGameUserConfigDir() {
 	String dir = _G(platform)->GetUserConfigDirectory();
 	if (Path::IsRelativePath(dir)) // relative dir is resolved relative to the game data dir
-		return FSLocation(_GP(ResPaths).DataDir, Path::ConcatPaths(_GP(ResPaths).DataDir, dir));
+		return FSLocation(_GP(ResPaths).DataDir, dir);
 	else if (_GP(usetup).local_user_conf) // directive to use game dir location
 		return FSLocation(_GP(ResPaths).DataDir);
 	// For absolute dir, we assume it's a special directory prepared for AGS engine
 	// and therefore amend it with a game own subdir
-	return FSLocation(dir, Path::ConcatPaths(dir, _GP(game).saveGameFolderName));
+	return FSLocation(dir, _GP(game).saveGameFolderName);
 }
 
 // A helper function that deduces a data directory either using default system location,
@@ -258,17 +258,17 @@ static FSLocation MakeGameDataDir(const String &default_dir, const String &user_
 	if (user_option.IsEmpty()) {
 		String dir = default_dir;
 		if (Path::IsRelativePath(dir)) // relative dir is resolved relative to the game data dir
-			return FSLocation(_GP(ResPaths).DataDir, Path::ConcatPaths(_GP(ResPaths).DataDir, dir));
+			return FSLocation(_GP(ResPaths).DataDir, dir);
 		// For absolute dir, we assume it's a special directory prepared for AGS engine
 		// and therefore amend it with a game own subdir
-		return FSLocation(dir, Path::ConcatPaths(dir, _GP(game).saveGameFolderName));
+		return FSLocation(dir, _GP(game).saveGameFolderName);
 	}
 	// If this location is set up by user config, then use it as is (resolving relative path if necessary)
 	String dir = user_option;
 	if (Path::IsSameOrSubDir(_GP(ResPaths).DataDir, dir)) // check if it's inside game dir
 		return FSLocation(_GP(ResPaths).DataDir, Path::MakeRelativePath(_GP(ResPaths).DataDir, dir));
 	dir = Path::MakeAbsolutePath(dir);
-	return FSLocation(dir, dir);
+	return FSLocation(dir);
 }
 
 FSLocation GetGameAppDataDir() {
@@ -290,8 +290,7 @@ bool ResolveScriptPath(const String &orig_sc_path, bool read_only, ResolvedPath
 	}
 
 	if (is_absolute) {
-		rp.FullPath = orig_sc_path;
-		debugC(::AGS::kDebugFilePath, "Full path detected");
+		rp = ResolvedPath(orig_sc_path);
 		return true;
 	}
 
@@ -361,28 +360,24 @@ bool ResolveScriptPath(const String &orig_sc_path, bool read_only, ResolvedPath
 
 	String full_path = String::FromFormat("%s%s", parent_dir.BaseDir.GetCStr(), child_path.GetCStr());
 	// don't allow write operations for relative paths outside game dir
+	ResolvedPath test_rp = ResolvedPath(parent_dir, child_path, alt_path);
 	if (!read_only) {
-		if (!Path::IsSameOrSubDir(parent_dir.FullDir, full_path)) {
+		if (!Path::IsSameOrSubDir(test_rp.Loc.FullDir, test_rp.FullPath)) {
 			debug_script_warn("Attempt to access file '%s' denied (outside of game directory)", sc_path.GetCStr());
 			return false;
 		}
 	}
 
-	rp.BaseDir = parent_dir.BaseDir;
-	rp.FullPath = full_path;
-	rp.AltPath = alt_path;
-
-	debugC(::AGS::kDebugFilePath, "Resolved path: %s", full_path.GetCStr());
-	if (!alt_path.IsEmpty())
-		debugC(::AGS::kDebugFilePath, "Alternative path: %s", alt_path.GetCStr());
-
+	rp = test_rp;
 	return true;
 }
 
 bool ResolveWritePathAndCreateDirs(const String &sc_path, ResolvedPath &rp) {
 	if (!ResolveScriptPath(sc_path, false, rp))
 		return false;
-	if (!Directory::CreateAllDirectories(rp.BaseDir, Path::GetDirectoryPath(rp.FullPath))) {
+
+	if (!rp.Loc.SubDir.IsEmpty() &&
+			!Directory::CreateAllDirectories(rp.Loc.BaseDir, rp.Loc.FullDir)) {
 		debug_script_warn("ResolveScriptPath: failed to create all subdirectories: %s", rp.FullPath.GetCStr());
 		return false;
 	}
diff --git a/engines/ags/engine/ac/game.cpp b/engines/ags/engine/ac/game.cpp
index cd74cc7ac0..261c718159 100644
--- a/engines/ags/engine/ac/game.cpp
+++ b/engines/ags/engine/ac/game.cpp
@@ -241,44 +241,46 @@ String get_save_game_path(int slotNum) {
 
 #if !AGS_PLATFORM_SCUMMVM
 // Convert a path possibly containing path tags into acceptable save path
-bool MakeSaveGameDir(const String &newFolder, ResolvedPath &rp) {
-	rp = ResolvedPath();
+bool MakeSaveGameDir(const String &newFolder, FSLocation &fsloc) {
+	fsloc = FSLocation();
 	// don't allow absolute paths
 	if (!Path::IsRelativePath(newFolder))
 		return false;
 
 	String base_dir;
-	String newSaveGameDir = FixSlashAfterToken(newFolder);
-
-	if (newSaveGameDir.CompareLeft(UserSavedgamesRootToken, UserSavedgamesRootToken.GetLength()) == 0) {
-		if (_G(saveGameParent).IsEmpty()) {
-			base_dir = PathFromInstallDir(_G(platform)->GetUserSavedgamesDirectory());
-			newSaveGameDir.ReplaceMid(0, UserSavedgamesRootToken.GetLength(), base_dir);
+	String sub_dir;
+
+	if (newFolder.CompareLeft(UserSavedgamesRootToken) == 0) {
+		// IMPORTANT: for compatibility reasons we support both cases:
+		// when token is followed by the path separator and when it is not, in which case it's assumed.
+		if (saveGameParent.IsEmpty()) {
+			base_dir = PathFromInstallDir(platform->GetUserSavedgamesDirectory());
+			sub_dir = newFolder.Mid(UserSavedgamesRootToken.GetLength());
 		} else {
 			// If there is a custom save parent directory, then replace
-			// not only root token, but also first subdirectory
-			newSaveGameDir.ClipSection('/', 0, 1); // TODO: Path helper function for this?
-			newSaveGameDir = Path::ConcatPaths(_G(saveGameParent), newSaveGameDir);
-			base_dir = _G(saveGameParent);
+			// not only root token, but also first subdirectory after the token
+			base_dir = saveGameParent;
+			sub_dir = Path::ConcatPaths(".", newFolder.Mid(UserSavedgamesRootToken.GetLength()));
+			sub_dir.ClipSection('/', 0, 1); // TODO: Path helper function for this?
 		}
+		fsloc = FSLocation(base_dir, sub_dir);
 	} else {
 		// Convert the path relative to installation folder into path relative to the
 		// safe save path with default name
-		if (_G(saveGameParent).IsEmpty()) {
-			base_dir = PathFromInstallDir(_G(platform)->GetUserSavedgamesDirectory());
-			newSaveGameDir = Path::ConcatPaths(Path::ConcatPaths(base_dir, _GP(game).saveGameFolderName), newFolder);
+		if (saveGameParent.IsEmpty()) {
+			base_dir = PathFromInstallDir(platform->GetUserSavedgamesDirectory());
+			sub_dir = Path::ConcatPaths(game.saveGameFolderName, newFolder);
 		} else {
-			base_dir = _G(saveGameParent);
-			newSaveGameDir = Path::ConcatPaths(_G(saveGameParent), newFolder);
+			base_dir = saveGameParent;
+			sub_dir = newFolder;
 		}
+		fsloc = FSLocation(base_dir, sub_dir);
 		// For games made in the safe-path-aware versions of AGS, report a warning
-		if (_GP(game).options[OPT_SAFEFILEPATHS]) {
+		if (game.options[OPT_SAFEFILEPATHS]) {
 			debug_script_warn("Attempt to explicitly set savegame location relative to the game installation directory ('%s') denied;\nPath will be remapped to the user documents directory: '%s'",
-			                  newFolder.GetCStr(), newSaveGameDir.GetCStr());
+				newFolder.GetCStr(), fsloc.FullDir.GetCStr());
 		}
 	}
-	rp.BaseDir = base_dir;
-	rp.FullPath = newSaveGameDir;
 	return true;
 }
 #endif
@@ -295,22 +297,21 @@ bool SetSaveGameDirectoryPath(const String &newFolder, bool explicit_path) {
 #if AGS_PLATFORM_SCUMMVM
 	return false;
 #else
-	if (!newFolder || newFolder[0] == 0)
-		newFolder = ".";
+	String newFolder = new_dir.IsEmpty() ? "." : new_dir;
 	String newSaveGameDir;
 	if (explicit_path) {
 		newSaveGameDir = PathFromInstallDir(newFolder);
 		if (!Directory::CreateDirectory(newSaveGameDir))
 			return false;
 	} else {
-		ResolvedPath rp;
-		if (!MakeSaveGameDir(newFolder, rp))
+		FSLocation fsloc;
+		if (!MakeSaveGameDir(newFolder, fsloc))
 			return false;
-		if (!Directory::CreateAllDirectories(rp.BaseDir, rp.FullPath)) {
-			debug_script_warn("SetSaveGameDirectory: failed to create all subdirectories: %s", rp.FullPath.GetCStr());
+		if (!Directory::CreateAllDirectories(fsloc.BaseDir, fsloc.FullDir)) {
+			debug_script_warn("SetSaveGameDirectory: failed to create all subdirectories: %s", fsloc.FullDir.GetCStr());
 			return false;
 		}
-		newSaveGameDir = rp.FullPath;
+		newSaveGameDir = fsloc.FullDir;
 	}
 
 	String newFolderTempFile = Path::ConcatPaths(newSaveGameDir, "agstmp.tmp");
@@ -318,7 +319,7 @@ bool SetSaveGameDirectoryPath(const String &newFolder, bool explicit_path) {
 		return false;
 
 	// copy the Restart Game file, if applicable
-	String restartGamePath = Path::ConcatPaths(_G(saveGameDirectory), get_save_game_filename(RESTART_POINT_SAVE_GAME_NUMBER));
+	String restartGamePath = Path::ConcatPaths(saveGameDirectory, get_save_game_filename(RESTART_POINT_SAVE_GAME_NUMBER));
 	Stream *restartGameFile = File::OpenFileRead(restartGamePath);
 	if (restartGameFile != nullptr) {
 		long fileSize = restartGameFile->GetLength();
@@ -333,7 +334,7 @@ bool SetSaveGameDirectoryPath(const String &newFolder, bool explicit_path) {
 		free(mbuffer);
 	}
 
-	_G(saveGameDirectory) = newSaveGameDir;
+	saveGameDirectory = newSaveGameDir;
 	return true;
 #endif
 }
diff --git a/engines/ags/engine/ac/path_helper.h b/engines/ags/engine/ac/path_helper.h
index 04e4d773bd..4900081ebd 100644
--- a/engines/ags/engine/ac/path_helper.h
+++ b/engines/ags/engine/ac/path_helper.h
@@ -33,7 +33,7 @@
 #ifndef AGS_ENGINE_AC_PATH_HELPER_H
 #define AGS_ENGINE_AC_PATH_HELPER_H
 
-#include "ags/shared/util/string.h"
+#include "ags/shared/util/path.h"
 
 namespace AGS3 {
 
@@ -57,12 +57,15 @@ String PathFromInstallDir(const String &path);
 // The meaning of this is that engine is only allowed to create
 // sub-path subdirectories, and only if secure path exists.
 struct FSLocation {
-	String BaseDir; // parent part of the full path that is not our responsibility
-	String FullDir; // full path to the directory
+	String BaseDir; // base directory, which we assume already exists; not our responsibility
+	String SubDir;  // sub-directory, relative to BaseDir
+	String FullDir; // full path to location
 	FSLocation() {}
 	FSLocation(const String &base) : BaseDir(base), FullDir(base) {
 	}
-	FSLocation(const String &base, const String &full) : BaseDir(base), FullDir(full) {
+	FSLocation(const String &base, const String &subdir)
+		: BaseDir(base), SubDir(subdir),
+		FullDir(AGS::Shared::Path::ConcatPaths(base, subdir)) {
 	}
 };
 // Makes sure that given system location is available, makes directories if have to (and if it's allowed to)
@@ -81,9 +84,16 @@ FSLocation GetGameUserDataDir();
 
 // ResolvedPath describes an actual location pointed by a user path (e.g. from script)
 struct ResolvedPath {
-	String BaseDir;  // base directory, which we assume already exists
+	FSLocation Loc;  // location (directory)
 	String FullPath; // full path, including filename
-	String AltPath;  // alternative full path, for backwards compatibility
+	String AltPath;  // alternative read-only full path, for backwards compatibility
+	ResolvedPath() = default;
+	ResolvedPath(const String & file, const String & alt = "")
+		: FullPath(file), AltPath(alt) {
+	}
+	ResolvedPath(const FSLocation & loc, const String & file, const String & alt = "")
+		: Loc(loc), FullPath(AGS::Shared::Path::ConcatPaths(loc.FullDir, file)), AltPath(alt) {
+	}
 };
 // Resolves a file path provided by user (e.g. script) into actual file path,
 // by substituting special keywords with actual platform-specific directory names.


Commit: 0ba468d1c92e99341b248bdcef3652b75c6f3587
    https://github.com/scummvm/scummvm/commit/0ba468d1c92e99341b248bdcef3652b75c6f3587
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:56-07:00

Commit Message:
AGS: CreateAllDirectories works explicitly with sub_dirs parameter

>From upstream 80f54e2c17830121bbe8b8501f22536c90b3f760

Changed paths:
    engines/ags/engine/ac/file.cpp
    engines/ags/engine/ac/game.cpp
    engines/ags/shared/util/directory.cpp
    engines/ags/shared/util/directory.h
    engines/ags/shared/util/path.h
    engines/ags/shared/util/string.cpp
    engines/ags/shared/util/string.h


diff --git a/engines/ags/engine/ac/file.cpp b/engines/ags/engine/ac/file.cpp
index eeb93a422a..acbe86d028 100644
--- a/engines/ags/engine/ac/file.cpp
+++ b/engines/ags/engine/ac/file.cpp
@@ -228,7 +228,7 @@ String PathFromInstallDir(const String &path) {
 }
 
 String PreparePathForWriting(const FSLocation &fsloc, const String &filename) {
-	if (Directory::CreateAllDirectories(fsloc.BaseDir, fsloc.FullDir))
+	if (Directory::CreateAllDirectories(fsloc.BaseDir, fsloc.SubDir))
 		return Path::ConcatPaths(fsloc.FullDir, filename);
 	return "";
 }
@@ -377,7 +377,7 @@ bool ResolveWritePathAndCreateDirs(const String &sc_path, ResolvedPath &rp) {
 		return false;
 
 	if (!rp.Loc.SubDir.IsEmpty() &&
-			!Directory::CreateAllDirectories(rp.Loc.BaseDir, rp.Loc.FullDir)) {
+			!Directory::CreateAllDirectories(rp.Loc.BaseDir, rp.Loc.SubDir)) {
 		debug_script_warn("ResolveScriptPath: failed to create all subdirectories: %s", rp.FullPath.GetCStr());
 		return false;
 	}
diff --git a/engines/ags/engine/ac/game.cpp b/engines/ags/engine/ac/game.cpp
index 261c718159..13158a6506 100644
--- a/engines/ags/engine/ac/game.cpp
+++ b/engines/ags/engine/ac/game.cpp
@@ -307,7 +307,7 @@ bool SetSaveGameDirectoryPath(const String &newFolder, bool explicit_path) {
 		FSLocation fsloc;
 		if (!MakeSaveGameDir(newFolder, fsloc))
 			return false;
-		if (!Directory::CreateAllDirectories(fsloc.BaseDir, fsloc.FullDir)) {
+		if (!Directory::CreateAllDirectories(fsloc.BaseDir, fsloc.SubDir)) {
 			debug_script_warn("SetSaveGameDirectory: failed to create all subdirectories: %s", fsloc.FullDir.GetCStr());
 			return false;
 		}
diff --git a/engines/ags/shared/util/directory.cpp b/engines/ags/shared/util/directory.cpp
index df5611fab1..aea9cd7086 100644
--- a/engines/ags/shared/util/directory.cpp
+++ b/engines/ags/shared/util/directory.cpp
@@ -39,27 +39,33 @@ bool CreateDirectory(const String &path) {
 	return Common::FSNode(path.GetCStr()).createDirectory();
 }
 
-bool CreateAllDirectories(const String &parent, const String &path) {
-	if (path == SAVE_FOLDER_PREFIX)
+bool CreateAllDirectories(const String &parent, const String &sub_dirs) {
+	if (sub_dirs == SAVE_FOLDER_PREFIX)
 		// ScummVM save folder doesn't need creating
 		return true;
 
-	if (!ags_directory_exists(parent.GetCStr()))
-		return false;
-	if (path.IsEmpty())
-		return true;
-	if (!Path::IsSameOrSubDir(parent, path))
-		return false;
+	if (parent.IsEmpty() || !ags_directory_exists(parent.GetCStr()))
+		return false; // no sense, or base dir not exist
+	if (sub_dirs.IsEmpty())
+		return true; // nothing to create, so fine
 
-	String sub_path = Path::MakeRelativePath(parent, path);
-	String make_path = parent;
-	std::vector<String> dirs = sub_path.Split('/');
-	for (const String &dir : dirs) {
-		if (dir.IsEmpty() || dir.Compare(".") == 0) continue;
-		make_path.AppendChar('/');
-		make_path.Append(dir);
+	String make_path = String::FromFormat("%s/", parent.GetCStr());
+	for (const char *sect = sub_dirs.GetCStr();
+		sect < sub_dirs.GetCStr() + sub_dirs.GetLength();) {
+		const char *cur = sect + 1;
+		for (; *cur && *cur != '/' && *cur != PATH_ALT_SEPARATOR; ++cur);
+		// Skip empty dirs (duplicated separators etc)
+		if ((cur - sect == 1) && (*cur == '.' || *cur == '/' || *cur == PATH_ALT_SEPARATOR)) {
+			sect = cur;
+			continue;
+		}
+		// In case of ".." just fail
+		if (strncmp(sect, "..", cur - sect) == 0)
+			return false;
+		make_path.Append(sect, cur - sect);
 		if (!CreateDirectory(make_path))
 			return false;
+		sect = cur;
 	}
 	return true;
 }
diff --git a/engines/ags/shared/util/directory.h b/engines/ags/shared/util/directory.h
index e544b299d2..0093193da2 100644
--- a/engines/ags/shared/util/directory.h
+++ b/engines/ags/shared/util/directory.h
@@ -42,9 +42,9 @@ namespace Directory {
 
 // Creates new directory (if it does not exist)
 bool   CreateDirectory(const String &path);
-// Makes sure all directories in the path are created. Parent path is
+// Makes sure all the sub-directories in the path are created. Parent path is
 // not touched, and function must fail if parent path is not accessible.
-bool   CreateAllDirectories(const String &parent, const String &path);
+bool   CreateAllDirectories(const String &parent, const String &sub_dirs);
 // Sets current working directory, returns the resulting path
 String SetCurrentDirectory(const String &path);
 // Gets current working directory
diff --git a/engines/ags/shared/util/path.h b/engines/ags/shared/util/path.h
index 91b92e8371..664beb8754 100644
--- a/engines/ags/shared/util/path.h
+++ b/engines/ags/shared/util/path.h
@@ -31,6 +31,8 @@
 
 #include "ags/shared/util/string.h"
 
+#define PATH_ALT_SEPARATOR ('\\')
+
 namespace AGS3 {
 namespace AGS {
 namespace Shared {
diff --git a/engines/ags/shared/util/string.cpp b/engines/ags/shared/util/string.cpp
index fd4c60e1e6..d79c28e00b 100644
--- a/engines/ags/shared/util/string.cpp
+++ b/engines/ags/shared/util/string.cpp
@@ -419,15 +419,28 @@ void String::Compact() {
 }
 
 void String::Append(const String &str) {
-	size_t length = str._len;
 	if (str._len > 0) {
-		ReserveAndShift(false, length);
-		memcpy(_cstr + _len, str._cstr, length);
-		_len += length;
+		ReserveAndShift(false, str._len);
+		memcpy(_cstr + _len, str._cstr, str._len);
+		_len += str._len;
 		_cstr[_len] = 0;
 	}
 }
 
+void String::Append(const char *cstr, size_t len) {
+	if (len == 0)
+		return;
+	// Test for null-terminator in the range
+	const char *ptr = cstr;
+	for (; *ptr && (size_t)(ptr - cstr) < len; ++ptr);
+	if ((size_t)(ptr - cstr) < len)
+		len = ptr - cstr;
+	ReserveAndShift(false, len);
+	memcpy(_cstr + _len, cstr, len);
+	_len += len;
+	_cstr[_len] = 0;
+}
+
 void String::AppendChar(char c) {
 	if (c) {
 		ReserveAndShift(false, 1);
diff --git a/engines/ags/shared/util/string.h b/engines/ags/shared/util/string.h
index 8d41d856c4..1d1c108c0c 100644
--- a/engines/ags/shared/util/string.h
+++ b/engines/ags/shared/util/string.h
@@ -281,8 +281,10 @@ public:
 	// Appends another string to this string
 	void    Append(const String &str);
 	void    Append(const char *cstr) {
-		String str = String::Wrapper(cstr); Append(str);
+		String str = String::Wrapper(cstr);
+		Append(str);
 	}
+	void    Append(const char *cstr, size_t len);
 	// Appends a single character
 	void    AppendChar(char c);
 	// Clip* methods decrease the string, removing defined part


Commit: 215139e49d1e8e650e0f4cf74e3e7ce9d62cf674
    https://github.com/scummvm/scummvm/commit/215139e49d1e8e650e0f4cf74e3e7ce9d62cf674
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:57-07:00

Commit Message:
AGS: Add fast-path for file lookup

>From upstream 8c088f61a3180d70591a98dcb36b189e5f6cb2e6

Changed paths:
    engines/ags/shared/util/misc.cpp


diff --git a/engines/ags/shared/util/misc.cpp b/engines/ags/shared/util/misc.cpp
index 6f0a774c26..e9ab5126c7 100644
--- a/engines/ags/shared/util/misc.cpp
+++ b/engines/ags/shared/util/misc.cpp
@@ -50,14 +50,6 @@
 */
 
 #include "ags/shared/core/platform.h"
-
-//include <sys/types.h>
-//include <sys/stat.h>
-#if !AGS_PLATFORM_OS_WINDOWS
-//include <dirent.h>
-//include <unistd.h>
-#endif
-
 #include "ags/lib/allegro.h" // file path functions
 #include "ags/shared/util/file.h"
 #include "ags/shared/util/stream.h"
@@ -70,8 +62,7 @@ using namespace AGS::Shared;
 // TODO: rewrite all this in a cleaner way perhaps, and move to our file or path utilities unit
 //
 
-#if !defined (AGS_CASE_SENSITIVE_FILESYSTEM)
-//include <string.h>
+#if !defined(AGS_CASE_SENSITIVE_FILESYSTEM)
 /* File Name Concatenator basically on Windows / DOS */
 char *ci_find_file(const char *dir_name, const char *file_name) {
 	char *diamond = NULL;
@@ -121,6 +112,15 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 		fix_filename_slashes(filename);
 	}
 
+	if (directory && filename) {
+		char buf[1024];
+		snprintf(buf, sizeof buf, "%s/%s", directory, filename);
+		lstat(buf, &statbuf);
+		if (S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode)) {
+			diamond = strdup(buf); goto out;
+		}
+	}
+
 	if (directory == nullptr) {
 		char *match = nullptr;
 		int   match_len = 0;
@@ -173,13 +173,14 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 				append_filename(diamond, directory, entry->d_name, strlen(directory) + strlen(entry->d_name) + 2);
 				break;
 			}
-		}
-	}
+}
+}
 	closedir(rough);
 
 	fchdir(dirfd(prevdir));
 	closedir(prevdir);
 
+out:;
 	free(directory);
 	free(filename);
 
@@ -187,7 +188,6 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 }
 #endif
 
-
 /* Case Insensitive fopen */
 Stream *ci_fopen(const char *file_name, FileOpenMode open_mode, FileWorkMode work_mode) {
 #if !defined (AGS_CASE_SENSITIVE_FILESYSTEM)


Commit: 8f3b1e4b0e80e5ba08aee4a9ac0fc1126f82b84e
    https://github.com/scummvm/scummvm/commit/8f3b1e4b0e80e5ba08aee4a9ac0fc1126f82b84e
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:57-07:00

Commit Message:
AGS: print all missing script imports for current script module

>From upstream 9a1c401ba97267947db5357b34db55e62b213f99

Changed paths:
    engines/ags/engine/script/cc_instance.cpp


diff --git a/engines/ags/engine/script/cc_instance.cpp b/engines/ags/engine/script/cc_instance.cpp
index fe71d254cf..e6e9a55bf7 100644
--- a/engines/ags/engine/script/cc_instance.cpp
+++ b/engines/ags/engine/script/cc_instance.cpp
@@ -1492,6 +1492,7 @@ bool ccInstance::ResolveScriptImports(PScript scri) {
 	}
 
 	resolved_imports = new int[numimports];
+	int errors = 0;
 	for (int i = 0; i < scri->numimports; ++i) {
 		if (scri->imports[i] == nullptr) {
 			resolved_imports[i] = -1;
@@ -1500,11 +1501,14 @@ bool ccInstance::ResolveScriptImports(PScript scri) {
 
 		resolved_imports[i] = _GP(simp).get_index_of(scri->imports[i]);
 		if (resolved_imports[i] < 0) {
-			cc_error("unresolved import '%s' in %s", scri->imports[i], scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>");
-			return false;
+			AGS::Shared::Debug::Printf(kDbgMsg_Info, "unresolved import '%s' in %s", scri->imports[i], scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>");
+			errors++;
 		}
 	}
-	return true;
+
+	if (errors > 0)
+		cc_error("unresolved imports, quitting.");
+	return errors == 0;
 }
 
 // TODO: it is possible to deduce global var's size at start with


Commit: ddb5daca4c4556cb5672d6841eb736562e4cc23c
    https://github.com/scummvm/scummvm/commit/ddb5daca4c4556cb5672d6841eb736562e4cc23c
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:57-07:00

Commit Message:
AGS: improve debug level and cc_error output for missing imports

>From upstream ca66d3cb69930c9f755da5fc26e173098169ae7d

Changed paths:
    engines/ags/engine/script/cc_instance.cpp


diff --git a/engines/ags/engine/script/cc_instance.cpp b/engines/ags/engine/script/cc_instance.cpp
index e6e9a55bf7..7cd8573092 100644
--- a/engines/ags/engine/script/cc_instance.cpp
+++ b/engines/ags/engine/script/cc_instance.cpp
@@ -1492,7 +1492,7 @@ bool ccInstance::ResolveScriptImports(PScript scri) {
 	}
 
 	resolved_imports = new int[numimports];
-	int errors = 0;
+	int errors = 0, last_err_idx = 0;
 	for (int i = 0; i < scri->numimports; ++i) {
 		if (scri->imports[i] == nullptr) {
 			resolved_imports[i] = -1;
@@ -1501,13 +1501,17 @@ bool ccInstance::ResolveScriptImports(PScript scri) {
 
 		resolved_imports[i] = _GP(simp).get_index_of(scri->imports[i]);
 		if (resolved_imports[i] < 0) {
-			AGS::Shared::Debug::Printf(kDbgMsg_Info, "unresolved import '%s' in %s", scri->imports[i], scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>");
+			Debug::Printf(kDbgMsg_Error, "unresolved import '%s' in '%s'", scri->imports[i], scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>");
 			errors++;
+			last_err_idx = i;
 		}
 	}
 
 	if (errors > 0)
-		cc_error("unresolved imports, quitting.");
+		cc_error("in %s: %d unresolved imports (last: %s)",
+			scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>",
+			errors,
+			scri->imports[last_err_idx]);
 	return errors == 0;
 }
 


Commit: 7238f9df51e7d60fdeb9c70f1267254bc745584a
    https://github.com/scummvm/scummvm/commit/7238f9df51e7d60fdeb9c70f1267254bc745584a
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:57-07:00

Commit Message:
AGS: Fix memory leak in ci_find_file error paths

>From upstream 1f7c216de21b7adaf96f8420bbbc10f124d2059b

Changed paths:
    engines/ags/shared/util/misc.cpp


diff --git a/engines/ags/shared/util/misc.cpp b/engines/ags/shared/util/misc.cpp
index e9ab5126c7..80244fe844 100644
--- a/engines/ags/shared/util/misc.cpp
+++ b/engines/ags/shared/util/misc.cpp
@@ -83,7 +83,7 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 }
 
 #else
-/* Case Insensitive File Find */
+/* Case Insensitive File Find - Only used on UNIX platforms */
 char *ci_find_file(const char *dir_name, const char *file_name) {
 	struct stat   statbuf;
 	struct dirent *entry = nullptr;
@@ -94,7 +94,7 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 	char *filename = nullptr;
 
 	if (dir_name == nullptr && file_name == nullptr)
-		return nullptr;
+		goto out;
 
 	if (dir_name != nullptr) {
 		directory = (char *)malloc(strlen(dir_name) + 1);
@@ -128,7 +128,7 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 
 		match = get_filename(filename);
 		if (match == nullptr)
-			return nullptr;
+			goto out;
 
 		match_len = strlen(match);
 		dir_len = (match - filename);
@@ -149,17 +149,17 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 
 	if ((prevdir = opendir(".")) == nullptr) {
 		fprintf(stderr, "ci_find_file: cannot open current working directory\n");
-		return nullptr;
+		goto out;
 	}
 
 	if (chdir(directory) == -1) {
 		fprintf(stderr, "ci_find_file: cannot change to directory: %s\n", directory);
-		return nullptr;
+		goto out;
 	}
 
 	if ((rough = opendir(directory)) == nullptr) {
 		fprintf(stderr, "ci_find_file: cannot open directory: %s\n", directory);
-		return nullptr;
+		goto out;
 	}
 
 	while ((entry = readdir(rough)) != nullptr) {
@@ -173,16 +173,16 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 				append_filename(diamond, directory, entry->d_name, strlen(directory) + strlen(entry->d_name) + 2);
 				break;
 			}
-}
-}
+		}
+	}
 	closedir(rough);
 
 	fchdir(dirfd(prevdir));
 	closedir(prevdir);
 
-out:;
-	free(directory);
-	free(filename);
+out:
+	if (directory) free(directory);
+	if (filename) free(filename);
 
 	return diamond;
 }


Commit: 284437b22ee303890139a40d0e9bd781315c2b07
    https://github.com/scummvm/scummvm/commit/284437b22ee303890139a40d0e9bd781315c2b07
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:57-07:00

Commit Message:
AGS: In ci_find_file, fix directory handle leak on error

>From upstream 3951d93e1125aac6aed8698dc2140e51a852358c

Changed paths:
    engines/ags/shared/util/misc.cpp


diff --git a/engines/ags/shared/util/misc.cpp b/engines/ags/shared/util/misc.cpp
index 80244fe844..d22ccb0413 100644
--- a/engines/ags/shared/util/misc.cpp
+++ b/engines/ags/shared/util/misc.cpp
@@ -83,7 +83,7 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 }
 
 #else
-/* Case Insensitive File Find - Only used on UNIX platforms */
+/* Case Sensitive File Find - only used on UNIX platforms */
 char *ci_find_file(const char *dir_name, const char *file_name) {
 	struct stat   statbuf;
 	struct dirent *entry = nullptr;
@@ -94,7 +94,7 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 	char *filename = nullptr;
 
 	if (dir_name == nullptr && file_name == nullptr)
-		goto out;
+		return nullptr;
 
 	if (dir_name != nullptr) {
 		directory = (char *)malloc(strlen(dir_name) + 1);
@@ -112,7 +112,14 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 		fix_filename_slashes(filename);
 	}
 
-	if (directory && filename) {
+	// the ".." check here prevents file system traversal -
+	// since only in this fast-path it's possible a potentially evil
+	// script could try to break out of the directories it's restricted
+	// to, whereas the latter chdir/opendir approach checks file by file
+	// in the directory. it's theoretically possible a valid filename
+	// could contain "..", but in that case it will just fallback to the
+	// slower method later on and succeed.
+	if (directory && filename && !strstr(filename, "..")) {
 		char buf[1024];
 		snprintf(buf, sizeof buf, "%s/%s", directory, filename);
 		lstat(buf, &statbuf);
@@ -154,12 +161,12 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 
 	if (chdir(directory) == -1) {
 		fprintf(stderr, "ci_find_file: cannot change to directory: %s\n", directory);
-		goto out;
+		goto out_pd;
 	}
 
 	if ((rough = opendir(directory)) == nullptr) {
 		fprintf(stderr, "ci_find_file: cannot open directory: %s\n", directory);
-		goto out;
+		goto out_pd;
 	}
 
 	while ((entry = readdir(rough)) != nullptr) {
@@ -177,10 +184,11 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 	}
 	closedir(rough);
 
+out_pd:;
 	fchdir(dirfd(prevdir));
 	closedir(prevdir);
 
-out:
+out:;
 	if (directory) free(directory);
 	if (filename) free(filename);
 


Commit: fabdbb1e201e7c3e764c87139b3ee877bd689bd3
    https://github.com/scummvm/scummvm/commit/fabdbb1e201e7c3e764c87139b3ee877bd689bd3
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:57-07:00

Commit Message:
AGS: In ci_find_file fix omission to check lstat return value

>From upstream f2ae56a5c07981a9470cfc85d5a9e8fd02151da6

This code is commented out for ScummVM in any case

Changed paths:
    engines/ags/shared/util/misc.cpp


diff --git a/engines/ags/shared/util/misc.cpp b/engines/ags/shared/util/misc.cpp
index d22ccb0413..90016ec2cc 100644
--- a/engines/ags/shared/util/misc.cpp
+++ b/engines/ags/shared/util/misc.cpp
@@ -119,11 +119,15 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 	// in the directory. it's theoretically possible a valid filename
 	// could contain "..", but in that case it will just fallback to the
 	// slower method later on and succeed.
-	if (directory && filename && !strstr(filename, "..")) {
+	if (filename && !strstr(filename, "..")) {
 		char buf[1024];
-		snprintf(buf, sizeof buf, "%s/%s", directory, filename);
-		lstat(buf, &statbuf);
-		if (S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode)) {
+		if (!directory && filename[0] == '/')
+			snprintf(buf, sizeof buf, "%s", filename);
+		else
+			snprintf(buf, sizeof buf, "%s/%s", directory ? directory : ".", filename);
+
+		if (lstat(buf, &statbuf) == 0 &&
+			(S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode))) {
 			diamond = strdup(buf); goto out;
 		}
 	}
@@ -170,9 +174,9 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 	}
 
 	while ((entry = readdir(rough)) != nullptr) {
-		lstat(entry->d_name, &statbuf);
-		if (S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode)) {
-			if (strcasecmp(filename, entry->d_name) == 0) {
+		if (strcasecmp(filename, entry->d_name) == 0) {
+			if (lstat(entry->d_name, &statbuf) == 0 &&
+				(S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode))) {
 #if AGS_PLATFORM_DEBUG
 				fprintf(stderr, "ci_find_file: Looked for %s in rough %s, found diamond %s.\n", filename, directory, entry->d_name);
 #endif // AGS_PLATFORM_DEBUG


Commit: bfd25ba440188328f192b72c9cb18f9215b29cb7
    https://github.com/scummvm/scummvm/commit/bfd25ba440188328f192b72c9cb18f9215b29cb7
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:58-07:00

Commit Message:
AGS: Reorganized ReadExtData functions into a DataExtReader class

>From upstream 462535ec21e2b184651d03d118b9cfd2258520e4

Changed paths:
    engines/ags/shared/game/main_game_file.cpp
    engines/ags/shared/game/room_file.cpp
    engines/ags/shared/game/room_file.h
    engines/ags/shared/game/room_file_base.cpp
    engines/ags/shared/game/tra_file.cpp
    engines/ags/shared/util/data_ext.cpp
    engines/ags/shared/util/data_ext.h


diff --git a/engines/ags/shared/game/main_game_file.cpp b/engines/ags/shared/game/main_game_file.cpp
index 0b8b153faf..3f7f1899ee 100644
--- a/engines/ags/shared/game/main_game_file.cpp
+++ b/engines/ags/shared/game/main_game_file.cpp
@@ -682,53 +682,60 @@ HGameFileError ReadSpriteFlags(LoadedGameEntities &ents, Stream *in, GameDataVer
 	return HGameFileError::None();
 }
 
-static HGameFileError ReadExtBlock(LoadedGameEntities &ents, Stream *in, const String &ext_id, soff_t block_len, GameDataVersion data_ver) {
-	// Add extensions here checking ext_id, which is an up to 16-chars name, for example:
-	// if (ext_id.CompareNoCase("GUI_NEWPROPS") == 0)
-	// {
-	//     // read new gui properties
-	// }
-	return new MainGameFileError(kMGFErr_ExtUnknown, String::FromFormat("Type: %s", ext_id.GetCStr()));
-}
+// GameDataExtReader reads main game data's extension blocks
+class GameDataExtReader : public DataExtReader {
+public:
+	GameDataExtReader(LoadedGameEntities &ents, GameDataVersion data_ver, Stream *in)
+		: DataExtReader(in, kDataExt_NumID8 | kDataExt_File64)
+		, _ents(ents)
+		, _dataVer(data_ver) {
+	}
+
+protected:
+	HError ReadBlock(int block_id, const String &ext_id,
+		soff_t block_len, bool &read_next) override;
 
-// This reader will process all blocks inside ReadExtBlock() function,
-// and read compatible data into the given LoadedGameEntities object.
-static LoadedGameEntities *reader_ents;
-static GameDataVersion reader_ver;
-HError ReadGameDataReader(Stream *in, int block_id, const String &ext_id,
+	LoadedGameEntities &_ents;
+	GameDataVersion _dataVer;
+};
+
+HError GameDataExtReader::ReadBlock(int block_id, const String &ext_id,
 		soff_t block_len, bool &read_next) {
-	return (HError)ReadExtBlock(*reader_ents, in, ext_id, block_len, reader_ver);
+    // Add extensions here checking ext_id, which is an up to 16-chars name, for example:
+    // if (ext_id.CompareNoCase("GUI_NEWPROPS") == 0)
+    // {
+    //     // read new gui properties
+    // }
+    return new MainGameFileError(kMGFErr_ExtUnknown, String::FromFormat("Type: %s", ext_id.GetCStr()));
 }
 
 HGameFileError ReadGameData(LoadedGameEntities &ents, Stream *in, GameDataVersion data_ver) {
 	GameSetupStruct &game = ents.Game;
-	reader_ents = &ents;
-	reader_ver = data_ver;
 
 	//-------------------------------------------------------------------------
 	// The classic data section.
 	//-------------------------------------------------------------------------
 	{
 		AlignedStream align_s(in, Shared::kAligned_Read);
-		_GP(game).GameSetupStructBase::ReadFromFile(&align_s);
+		game.GameSetupStructBase::ReadFromFile(&align_s);
 	}
 
-	Debug::Printf(kDbgMsg_Info, "Game title: '%s'", _GP(game).gamename);
+	Debug::Printf(kDbgMsg_Info, "Game title: '%s'", game.gamename);
 
-	if (_GP(game).GetGameRes().IsNull())
+	if (game.GetGameRes().IsNull())
 		return new MainGameFileError(kMGFErr_InvalidNativeResolution);
 
-	_GP(game).read_savegame_info(in, data_ver);
-	_GP(game).read_font_infos(in, data_ver);
+	game.read_savegame_info(in, data_ver);
+	game.read_font_infos(in, data_ver);
 	HGameFileError err = ReadSpriteFlags(ents, in, data_ver);
 	if (!err)
 		return err;
-	_GP(game).ReadInvInfo_Aligned(in);
-	err = _GP(game).read_cursors(in, data_ver);
+	game.ReadInvInfo_Aligned(in);
+	err = game.read_cursors(in, data_ver);
 	if (!err)
 		return err;
-	_GP(game).read_interaction_scripts(in, data_ver);
-	_GP(game).read_words_dictionary(in);
+	game.read_interaction_scripts(in, data_ver);
+	game.read_words_dictionary(in);
 
 	if (game.load_compiled_script) {
 		ents.GlobalScript.reset(ccScript::CreateFromStream(in));
@@ -750,16 +757,16 @@ HGameFileError ReadGameData(LoadedGameEntities &ents, Stream *in, GameDataVersio
 		in->Seek(count * 0x204);
 	}
 
-	_GP(game).read_characters(in, data_ver);
-	_GP(game).read_lipsync(in, data_ver);
-	_GP(game).read_messages(in, data_ver);
+	game.read_characters(in, data_ver);
+	game.read_lipsync(in, data_ver);
+	game.read_messages(in, data_ver);
 
 	ReadDialogs(ents.Dialogs, ents.OldDialogScripts, ents.OldDialogSources, ents.OldSpeechLines,
-	            in, data_ver, _GP(game).numdialog);
+		in, data_ver, game.numdialog);
 	HError err2 = GUI::ReadGUI(_GP(guis), in);
 	if (!err2)
 		return new MainGameFileError(kMGFErr_GameEntityFailed, err2);
-	_GP(game).numgui = _GP(guis).size();
+	game.numgui = _GP(guis).size();
 
 	if (data_ver >= kGameVersion_260) {
 		err = ReadPlugins(ents.PluginInfos, in);
@@ -767,13 +774,13 @@ HGameFileError ReadGameData(LoadedGameEntities &ents, Stream *in, GameDataVersio
 			return err;
 	}
 
-	err = _GP(game).read_customprops(in, data_ver);
+	err = game.read_customprops(in, data_ver);
 	if (!err)
 		return err;
-	err = _GP(game).read_audio(in, data_ver);
+	err = game.read_audio(in, data_ver);
 	if (!err)
 		return err;
-	_GP(game).read_room_names(in, data_ver);
+	game.read_room_names(in, data_ver);
 
 	if (data_ver <= kGameVersion_350)
 		return HGameFileError::None();
@@ -781,9 +788,8 @@ HGameFileError ReadGameData(LoadedGameEntities &ents, Stream *in, GameDataVersio
 	//-------------------------------------------------------------------------
 	// All the extended data, for AGS > 3.5.0.
 	//-------------------------------------------------------------------------
-
-	HError ext_err = ReadExtData(ReadGameDataReader,
-		kDataExt_NumID8 | kDataExt_File64, in);
+	GameDataExtReader reader(ents, data_ver, in);
+	HError ext_err = reader.Read();
 	return ext_err ? HGameFileError::None() : new MainGameFileError(kMGFErr_ExtListFailed, ext_err);
 }
 
diff --git a/engines/ags/shared/game/room_file.cpp b/engines/ags/shared/game/room_file.cpp
index fb1aa5bee6..a663bb9b5d 100644
--- a/engines/ags/shared/game/room_file.cpp
+++ b/engines/ags/shared/game/room_file.cpp
@@ -456,23 +456,43 @@ HError ReadRoomBlock(RoomStruct *room, Stream *in, RoomFileBlock block, const St
 		String::FromFormat("Type: %s", ext_id.GetCStr()));
 }
 
-// This reader will process all blocks inside ReadRoomBlock() function,
-// and read compatible data into the given RoomStruct
-static RoomStruct *reader_room;
-static RoomFileVersion reader_data_ver;
-
-static HError ReadRoomDataReader(Stream *in, int block_id,
-	const String &ext_id, soff_t block_len, bool &read_next) {
-	return (HError)ReadRoomBlock(reader_room, in, (RoomFileBlock)block_id, ext_id, block_len, reader_data_ver);
-}
+// RoomBlockReader reads whole room data, block by block
+class RoomBlockReader : public DataExtReader {
+public:
+	RoomBlockReader(RoomStruct *room, RoomFileVersion data_ver, Stream *in)
+		: DataExtReader(in,
+			kDataExt_NumID8 | ((data_ver < kRoomVersion_350) ? kDataExt_File32 : kDataExt_File64))
+		, _room(room)
+		, _dataVer(data_ver) {
+	}
+
+	// Helper function that extracts legacy room script
+	HError ReadRoomScript(String &script) {
+		HError err = FindOne(kRoomFblk_Script);
+		if (!err)
+			return err;
+		char *buf = nullptr;
+		err = ReadScriptBlock(buf, _in, _dataVer);
+		script = buf;
+		delete buf;
+		return err;
+	}
+
+private:
+	HError ReadBlock(int block_id, const String &ext_id,
+		soff_t block_len, bool &read_next) override {
+		return ReadRoomBlock(_room, _in, (RoomFileBlock)block_id, ext_id, block_len, _dataVer);
+	}
+
+	RoomStruct *_room{};
+	RoomFileVersion _dataVer{};
+};
+
 
 HRoomFileError ReadRoomData(RoomStruct *room, Stream *in, RoomFileVersion data_ver) {
 	room->DataVersion = data_ver;
-	reader_data_ver = data_ver;
-	reader_room = room;
-
-	HError err = ReadExtData(ReadRoomDataReader,
-		kDataExt_NumID8 | ((data_ver < kRoomVersion_350) ? kDataExt_File32 : kDataExt_File64), in);
+	RoomBlockReader reader(room, data_ver, in);
+	HError err = reader.Read();
 	return err ? HRoomFileError::None() :
 		new RoomFileError(kRoomFileErr_BlockListFailed, err);
 }
@@ -627,15 +647,11 @@ HError ExtractScriptTextReader(Stream *in, int block_id,
 }
 
 HRoomFileError ExtractScriptText(String &script, Stream *in, RoomFileVersion data_ver) {
-	// This reader would only process kRoomFblk_Script and exit as soon as one is found
-	reader_script = &script;
-	reader_ver = data_ver;
-
-	HError err = ReadExtData(ExtractScriptTextReader,
-		kDataExt_NumID8 | ((data_ver < kRoomVersion_350) ? kDataExt_File32 : kDataExt_File64), in);
-	if (err && script.IsEmpty())
-		new RoomFileError(kRoomFileErr_BlockNotFound);
-	return err ? HRoomFileError::None() : new RoomFileError(kRoomFileErr_BlockListFailed, err);
+	RoomBlockReader reader(nullptr, data_ver, in);
+	HError err = reader.ReadRoomScript(script);
+	if (!err)
+		new RoomFileError(kRoomFileErr_BlockListFailed, err);
+	return HRoomFileError::None();
 }
 
 void WriteInteractionScripts(const InteractionScripts *interactions, Stream *out) {
diff --git a/engines/ags/shared/game/room_file.h b/engines/ags/shared/game/room_file.h
index a52df6a59c..4eead0f868 100644
--- a/engines/ags/shared/game/room_file.h
+++ b/engines/ags/shared/game/room_file.h
@@ -128,14 +128,7 @@ HRoomFileError WriteRoomData(const RoomStruct *room, Stream *out, RoomFileVersio
 // Reads room data header using stream assigned to RoomDataSource;
 // tests and saves its format index if successful
 HRoomFileError ReadRoomHeader(RoomDataSource &src);
-// Opens next room block from the stream, fills in its identifier and length on success
-HRoomFileError OpenNextRoomBlock(Stream *in, RoomFileVersion data_ver, RoomFileBlock &block_id, String &ext_id, soff_t &block_len);
-// Type of function that reads single room block and tells whether to continue reading
-typedef HError(*PfnReadRoomBlock)(Stream * in, RoomFileBlock block_id, const String & ext_id,
-	soff_t block_len, RoomFileVersion data_ver, bool &read_next);
-// Parses room file, passing each found block into callback; does not read any actual data itself
-HRoomFileError ReadRoomData(PfnReadRoomBlock reader, Stream *in, RoomFileVersion data_ver);
-// Type of function that writes single room block.
+
 typedef void(*PfnWriteRoomBlock)(const RoomStruct *room, Stream *out);
 // Writes room block with a new-style string id
 void WriteRoomBlock(const RoomStruct *room, const String &ext_id, PfnWriteRoomBlock writer, Stream *out);
diff --git a/engines/ags/shared/game/room_file_base.cpp b/engines/ags/shared/game/room_file_base.cpp
index b84b4b0ed0..cf507c2c82 100644
--- a/engines/ags/shared/game/room_file_base.cpp
+++ b/engines/ags/shared/game/room_file_base.cpp
@@ -107,21 +107,6 @@ String GetRoomBlockName(RoomFileBlock id) {
 	return "unknown";
 }
 
-// This reader will delegate block reading to the provided user function
-static PfnReadRoomBlock reader_reader;
-static RoomFileVersion reader_ver;
-static HError ReadRoomDataReader(Stream *in, int block_id, const String &ext_id,
-		soff_t block_len, bool &read_next) {
-	return reader_reader(in, (RoomFileBlock)block_id, ext_id, block_len, reader_ver, read_next);
-}
-
-HRoomFileError ReadRoomData(PfnReadRoomBlock reader, Stream *in, RoomFileVersion data_ver) {
-	reader_reader = reader;
-	reader_ver = data_ver;
-	HError err = ReadExtData(ReadRoomDataReader,
-		kDataExt_NumID8 | ((data_ver < kRoomVersion_350) ? kDataExt_File32 : kDataExt_File64), in);
-	return err ? HRoomFileError::None() : new RoomFileError(kRoomFileErr_BlockListFailed, err);
-}
 
 static PfnWriteRoomBlock writer_writer;
 static const RoomStruct *writer_room;
diff --git a/engines/ags/shared/game/tra_file.cpp b/engines/ags/shared/game/tra_file.cpp
index f023c827a3..faf0bae2c9 100644
--- a/engines/ags/shared/game/tra_file.cpp
+++ b/engines/ags/shared/game/tra_file.cpp
@@ -119,27 +119,41 @@ HError ReadTraBlock(Translation &tra, Stream *in, TraFileBlock block, const Stri
 		String::FromFormat("Type: %s", ext_id.GetCStr()));
 }
 
-static Translation *reader_tra;
-HError TestTraGameIDReader(Stream *in, int block_id, const String &ext_id,
-		soff_t block_len, bool &read_next) {
-	if (block_id == kTraFblk_GameID) {
-		read_next = false;
-		return ReadTraBlock(*reader_tra, in, (TraFileBlock)block_id, ext_id, block_len);
+// TRABlockReader reads whole TRA data, block by block
+class TRABlockReader : public DataExtReader {
+public:
+	TRABlockReader(Translation &tra, Stream *in)
+		: DataExtReader(in, kDataExt_NumID32 | kDataExt_File32)
+		, _tra(tra) {
 	}
-	in->Seek(block_len); // skip block
-	return HError::None();
-}
+
+	// Reads only the Game ID block and stops
+	HError ReadGameID() {
+		HError err = FindOne(kTraFblk_GameID);
+		if (!err)
+			return err;
+		return ReadTraBlock(_tra, _in, kTraFblk_GameID, "", _block_len);
+	}
+
+private:
+	HError ReadBlock(int block_id, const String &ext_id,
+		soff_t block_len, bool &read_next) override {
+		return ReadTraBlock(_tra, _in, (TraFileBlock)block_id, ext_id, block_len);
+	}
+
+	Translation &_tra;
+};
+
 
 HError TestTraGameID(int game_uid, const String &game_name, Stream *in) {
 	HError err = OpenTraFile(in);
 	if (!err)
 		return err;
 
-	// This reader would only process kTraFblk_GameID and exit as soon as one is found
 	Translation tra;
-	reader_tra = &tra;
+	TRABlockReader reader(tra, in);
+	err = reader.ReadGameID();
 
-	err = ReadExtData(TestTraGameIDReader, kDataExt_NumID32 | kDataExt_File32, in);
 	if (!err)
 		return err;
 	// Test the identifiers, if they are not present then skip the test
@@ -150,20 +164,13 @@ HError TestTraGameID(int game_uid, const String &game_name, Stream *in) {
 	return HError::None();
 }
 
-// This reader will process all blocks inside ReadTraBlock() function,
-// and read compatible data into the given Translation object
-HError ReadTraDataReader(Stream *in, int block_id, const String &ext_id,
-		soff_t block_len, bool &read_next) {
-	return ReadTraBlock(*reader_tra, in, (TraFileBlock)block_id, ext_id, block_len);
-}
-
 HError ReadTraData(Translation &tra, Stream *in) {
-	reader_tra = &tra;
 	HError err = OpenTraFile(in);
 	if (!err)
 		return err;
 
-	return ReadExtData(ReadTraDataReader, kDataExt_NumID32 | kDataExt_File32, in);
+	TRABlockReader reader(tra, in);
+	return reader.Read();
 }
 
 // TODO: perhaps merge with encrypt/decrypt utilities
diff --git a/engines/ags/shared/util/data_ext.cpp b/engines/ags/shared/util/data_ext.cpp
index 63a59389a7..b7c9d0a733 100644
--- a/engines/ags/shared/util/data_ext.cpp
+++ b/engines/ags/shared/util/data_ext.cpp
@@ -40,73 +40,81 @@ String GetDataExtErrorText(DataExtErrorType err) {
 	return "Unknown error.";
 }
 
-HError OpenExtBlock(Stream *in, int flags, int &block_id, String &ext_id, soff_t &block_len) {
+HError DataExtParser::OpenBlock() {
 	//    - 1 or 4 bytes - an old-style unsigned numeric ID:
 	//               where 0 would indicate following string ID,
 	//               and -1 indicates end of the block list.
 	//    - 16 bytes - string ID of an extension (if numeric ID is 0).
 	//    - 4 or 8 bytes - length of extension data, in bytes.
-	block_id = ((flags & kDataExt_NumID32) != 0) ?
-		in->ReadInt32() :
-		in->ReadInt8();
+	_block_id = ((_flags & kDataExt_NumID32) != 0) ?
+		_in->ReadInt32() :
+		_in->ReadInt8();
 
-	if (block_id < 0)
+	if (_block_id < 0)
 		return HError::None(); // end of list
-	if (in->EOS())
+	if (_in->EOS())
 		return new DataExtError(kDataExtErr_UnexpectedEOF);
 
-	if (block_id > 0) { // old-style block identified by a numeric id
-		block_len = ((flags & kDataExt_File64) != 0) ? in->ReadInt64() : in->ReadInt32();
+	if (_block_id > 0) { // old-style block identified by a numeric id
+		_block_len = ((_flags & kDataExt_File64) != 0) ? _in->ReadInt64() : _in->ReadInt32();
+		_ext_id = GetOldBlockName(_block_id);
 	} else { // new style block identified by a string id
-		ext_id = String::FromStreamCount(in, 16);
-		block_len = in->ReadInt64();
+		_ext_id = String::FromStreamCount(_in, 16);
+		_block_len = _in->ReadInt64();
 	}
+	_block_start = _in->GetPosition();
 	return HError::None();
 }
 
-HError ReadExtData(PfnReadExtBlock reader, int flags, Stream *in) {
-	while (!in->EOS()) {
-		// First try open the next block
-		int block_id;
-		String ext_id;
-		soff_t block_len;
-		HError err = OpenExtBlock(in, flags, block_id, ext_id, block_len);
-		if (!err)
-			return err;
-		if (block_id < 0)
-			break; // end of list
-		if (ext_id.IsEmpty()) // we may need some name for the messages
-			ext_id.Format("id:%d", block_id);
-
-		// Now call the reader function to read current block's data
-		soff_t block_end = in->GetPosition() + block_len;
-		bool read_next = true;
-		err = reader(in, block_id, ext_id, block_len, read_next);
-		if (!err)
-			return err;
+void DataExtParser::SkipBlock() {
+	if (_block_id >= 0)
+		_in->Seek(_block_len);
+}
 
-		soff_t cur_pos = in->GetPosition();
+HError DataExtParser::PostAssert() {
+	const soff_t cur_pos = _in->GetPosition();
+	const soff_t block_end = _block_start + _block_len;
+	if (cur_pos > block_end) {
+		return new DataExtError(kDataExtErr_BlockDataOverlapping,
+			String::FromFormat("Block: '%s', expected to end at offset: %lld, finished reading at %lld.",
+				_ext_id.GetCStr(), block_end, cur_pos));
+	} else if (cur_pos < block_end) {
+		Debug::Printf(kDbgMsg_Warn, "WARNING: data blocks nonsequential, block '%s' expected to end at %lld, finished reading at %lld",
+			_ext_id.GetCStr(), block_end, cur_pos);
+		_in->Seek(block_end, Shared::kSeekBegin);
+	}
+	return HError::None();
+}
 
-		// WORKAROUND: For at least the MMM games, the translation
-		// files' first block length is incorrect by one byte
-		if (cur_pos == (block_end + 1) && cur_pos < 100)
-			cur_pos = block_end;
+HError DataExtParser::FindOne(int id) {
+	if (id <= 0) return new DataExtError(kDataExtErr_BlockNotFound);
 
-		// Finally test that we did not read too much or too little
-		if (cur_pos > block_end) {
-			return new DataExtError(kDataExtErr_BlockDataOverlapping,
-				String::FromFormat("Block: '%s', expected to end at offset: %lld, finished reading at %lld.",
-					ext_id.GetCStr(), block_end, cur_pos));
-		} else if (cur_pos < block_end) {
-			Debug::Printf(kDbgMsg_Warn, "WARNING: room data blocks nonsequential, block '%s' expected to end at %lld, finished reading at %lld",
-				ext_id.GetCStr(), block_end, cur_pos);
-			in->Seek(block_end, Shared::kSeekBegin);
-		}
+	HError err = HError::None();
+	for (err = OpenBlock(); err && !AtEnd(); err = OpenBlock()) {
+		if (id == _block_id)
+			return HError::None();
+		_in->Seek(_block_len); // skip it
+	}
+	if (!err)
+		return err;
+	return new DataExtError(kDataExtErr_BlockNotFound);
+}
 
-		if (!read_next)
-			break; // reader requested a stop, do so
+HError DataExtReader::Read() {
+	HError err = HError::None();
+	bool read_next = true;
+	for (err = OpenBlock(); err && !AtEnd() && read_next; err = OpenBlock()) {
+		// Call the reader function to read current block's data
+		read_next = true;
+		err = ReadBlock(_block_id, _ext_id, _block_len, read_next);
+		if (!err)
+			return err;
+		// Test that we did not read too much or too little
+		err = PostAssert();
+		if (!err)
+			return err;
 	}
-	return HError::None();
+	return err;
 }
 
 // Generic function that saves a block and automatically adds its size into header
diff --git a/engines/ags/shared/util/data_ext.h b/engines/ags/shared/util/data_ext.h
index 7545273c2f..0a1c3517e6 100644
--- a/engines/ags/shared/util/data_ext.h
+++ b/engines/ags/shared/util/data_ext.h
@@ -77,6 +77,7 @@ enum DataExtFlags {
 enum DataExtErrorType {
 	kDataExtErr_NoError,
 	kDataExtErr_UnexpectedEOF,
+	kDataExtErr_BlockNotFound,
 	kDataExtErr_BlockDataOverlapping
 };
 
@@ -84,14 +85,69 @@ String GetDataExtErrorText(DataExtErrorType err);
 typedef TypedCodeError<DataExtErrorType, GetDataExtErrorText> DataExtError;
 
 
-// Tries to opens a next block from the stream, fills in identifier and length on success
-HError OpenExtBlock(Stream *in, int flags, int &block_id, String &ext_id, soff_t &block_len);
-// Type of function that reads a single data block and tells whether to continue reading
-typedef HError(*PfnReadExtBlock)(Stream *in, int block_id, const String &ext_id,
-	soff_t block_len, bool &read_next);
-// Parses stream as a block list, passing each found block into callback;
-// does not read any actual data itself
-HError ReadExtData(PfnReadExtBlock reader, int flags, Stream *in);
+// DataExtReader parses a generic extendable block list and
+// does format checks, but does not read any data itself.
+// Use it to open blocks, and assert reading correctness.
+class DataExtParser {
+public:
+	DataExtParser(Stream *in, int flags) : _in(in), _flags(flags) {
+	}
+	virtual ~DataExtParser() = default;
+
+	// Returns the conventional string ID for an old-style block with numeric ID
+	virtual String GetOldBlockName(int blockId) const {
+		return String::FromFormat("id:%d", blockId);
+	}
+
+	// Gets a stream
+	inline Stream *GetStream() {
+		return _in;
+	}
+	// Tells if the end of the block list was reached
+	inline bool AtEnd() const {
+		return _block_id < 0;
+	}
+	// Tries to opens a next standard block from the stream,
+	// fills in identifier and length on success
+	HError OpenBlock();
+	// Skips current block
+	void   SkipBlock();
+	// Asserts current stream position after a block was read
+	HError PostAssert();
+	// Parses a block list in search for a particular block,
+	// if found opens it.
+	HError FindOne(int id);
+
+protected:
+	Stream *_in = nullptr;
+	int _flags = 0;
+
+	int _block_id = -1;
+	String _ext_id;
+	soff_t _block_start = 0;
+	soff_t _block_len = 0;
+};
+
+// DataExtReader is a virtual base class of a block list reader; provides
+// a helper method for reading all the blocks one by one, but leaves data
+// reading for the child classes. A child class must override ReadBlock method.
+// TODO: don't extend Parser, but have it as a member?
+class DataExtReader : protected DataExtParser {
+public:
+	virtual ~DataExtReader() = default;
+
+	// Parses a block list, calls ReadBlock for each found block
+	HError Read();
+
+protected:
+	DataExtReader(Stream *in, int flags) : DataExtParser(in, flags) {
+	}
+	// Reads a single data block and tell whether to continue reading;
+	// default implementation skips the block
+	virtual HError ReadBlock(int block_id, const String &ext_id,
+		soff_t block_len, bool &read_next) = 0;
+};
+
 
 // Type of function that writes a single data block.
 typedef void(*PfnWriteExtBlock)(Stream *out);


Commit: ddf30d3234b3da6889a9b0c76da2a43e2713ba46
    https://github.com/scummvm/scummvm/commit/ddf30d3234b3da6889a9b0c76da2a43e2713ba46
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:58-07:00

Commit Message:
AGS: implement GetOldBlockName() in RoomBlockReader, TRABlockReader

>From upstream 838605f7689febdf8174083d9dace140a6e07e19

Changed paths:
    engines/ags/shared/game/room_file.cpp
    engines/ags/shared/game/tra_file.cpp


diff --git a/engines/ags/shared/game/room_file.cpp b/engines/ags/shared/game/room_file.cpp
index a663bb9b5d..0091f62b03 100644
--- a/engines/ags/shared/game/room_file.cpp
+++ b/engines/ags/shared/game/room_file.cpp
@@ -479,6 +479,10 @@ public:
 	}
 
 private:
+	String GetOldBlockName(int block_id) const override {
+		return GetRoomBlockName((RoomFileBlock)block_id);
+	}
+
 	HError ReadBlock(int block_id, const String &ext_id,
 		soff_t block_len, bool &read_next) override {
 		return ReadRoomBlock(_room, _in, (RoomFileBlock)block_id, ext_id, block_len, _dataVer);
diff --git a/engines/ags/shared/game/tra_file.cpp b/engines/ags/shared/game/tra_file.cpp
index faf0bae2c9..f2a863cb2e 100644
--- a/engines/ags/shared/game/tra_file.cpp
+++ b/engines/ags/shared/game/tra_file.cpp
@@ -136,6 +136,10 @@ public:
 	}
 
 private:
+	String GetOldBlockName(int block_id) const override {
+		return GetTraBlockName((TraFileBlock)block_id);
+	}
+
 	HError ReadBlock(int block_id, const String &ext_id,
 		soff_t block_len, bool &read_next) override {
 		return ReadTraBlock(_tra, _in, (TraFileBlock)block_id, ext_id, block_len);


Commit: 7b42f9c35087f0a16e423d62483130e3aaebcb09
    https://github.com/scummvm/scummvm/commit/7b42f9c35087f0a16e423d62483130e3aaebcb09
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:58-07:00

Commit Message:
AGS: Optional block over-read leeway, for TRABlockReader

>From upstream 92c0844519653507ec979037d46a58a60ea93032

Changed paths:
    engines/ags/shared/game/tra_file.cpp
    engines/ags/shared/util/data_ext.cpp
    engines/ags/shared/util/data_ext.h


diff --git a/engines/ags/shared/game/tra_file.cpp b/engines/ags/shared/game/tra_file.cpp
index f2a863cb2e..fe42175685 100644
--- a/engines/ags/shared/game/tra_file.cpp
+++ b/engines/ags/shared/game/tra_file.cpp
@@ -140,6 +140,12 @@ private:
 		return GetTraBlockName((TraFileBlock)block_id);
 	}
 
+	soff_t GetOverLeeway(int block_id) const override {
+		// TRA files made by pre-3.0 editors have a block length miscount by 1 byte
+		if (block_id == kTraFblk_GameID) return 1;
+		return 0;
+	}
+
 	HError ReadBlock(int block_id, const String &ext_id,
 		soff_t block_len, bool &read_next) override {
 		return ReadTraBlock(_tra, _in, (TraFileBlock)block_id, ext_id, block_len);
diff --git a/engines/ags/shared/util/data_ext.cpp b/engines/ags/shared/util/data_ext.cpp
index b7c9d0a733..f7115aa591 100644
--- a/engines/ags/shared/util/data_ext.cpp
+++ b/engines/ags/shared/util/data_ext.cpp
@@ -75,9 +75,12 @@ HError DataExtParser::PostAssert() {
 	const soff_t cur_pos = _in->GetPosition();
 	const soff_t block_end = _block_start + _block_len;
 	if (cur_pos > block_end) {
-		return new DataExtError(kDataExtErr_BlockDataOverlapping,
-			String::FromFormat("Block: '%s', expected to end at offset: %lld, finished reading at %lld.",
-				_ext_id.GetCStr(), block_end, cur_pos));
+		String err = String::FromFormat("Block: '%s', expected to end at offset: %lld, finished reading at %lld.",
+			_ext_id.GetCStr(), block_end, cur_pos);
+		if (cur_pos <= block_end + GetOverLeeway(_block_id))
+			Debug::Printf(kDbgMsg_Warn, err);
+		else
+			return new DataExtError(kDataExtErr_BlockDataOverlapping, err);
 	} else if (cur_pos < block_end) {
 		Debug::Printf(kDbgMsg_Warn, "WARNING: data blocks nonsequential, block '%s' expected to end at %lld, finished reading at %lld",
 			_ext_id.GetCStr(), block_end, cur_pos);
diff --git a/engines/ags/shared/util/data_ext.h b/engines/ags/shared/util/data_ext.h
index 0a1c3517e6..5db288558f 100644
--- a/engines/ags/shared/util/data_ext.h
+++ b/engines/ags/shared/util/data_ext.h
@@ -99,6 +99,12 @@ public:
 		return String::FromFormat("id:%d", blockId);
 	}
 
+	// Provides a leeway for over-reading (reading past the reported block length):
+	// the parser will not error if the mistake is in this range of bytes
+	virtual soff_t GetOverLeeway(int block_id) const {
+		return 0;
+	}
+
 	// Gets a stream
 	inline Stream *GetStream() {
 		return _in;


Commit: f19dd1c506e40b5d86d1163357070c4a0bce1497
    https://github.com/scummvm/scummvm/commit/f19dd1c506e40b5d86d1163357070c4a0bce1497
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:58-07:00

Commit Message:
AGS: few cast fixes

>From upstream 008b2d2c61420f6f2c996317a0eb56a49e708095

Changed paths:
    engines/ags/engine/ac/timer.cpp
    engines/ags/shared/util/geometry.cpp
    engines/ags/shared/util/string.cpp


diff --git a/engines/ags/engine/ac/timer.cpp b/engines/ags/engine/ac/timer.cpp
index a7c8d889c6..cd3b1a9e5e 100644
--- a/engines/ags/engine/ac/timer.cpp
+++ b/engines/ags/engine/ac/timer.cpp
@@ -61,7 +61,7 @@ void WaitForNextFrame() {
 	if (frameDuration <= std::chrono::milliseconds::zero()) {
 		_G(next_frame_timestamp) = now;
 		// suspend while the game is being switched out
-		while (_G(game_update_suspend) > 0) {
+		while (_G(game_update_suspend)) {
 			sys_evt_process_pending();
 			_G(platform)->YieldCPU();
 		}
@@ -81,7 +81,7 @@ void WaitForNextFrame() {
 	_G(next_frame_timestamp) += frameDuration;
 
 	// suspend while the game is being switched out
-	while (_G(game_update_suspend) > 0) {
+	while (_G(game_update_suspend)) {
 		sys_evt_process_pending();
 		_G(platform)->YieldCPU();
 	}
diff --git a/engines/ags/shared/util/geometry.cpp b/engines/ags/shared/util/geometry.cpp
index f38492667a..93baef1a46 100644
--- a/engines/ags/shared/util/geometry.cpp
+++ b/engines/ags/shared/util/geometry.cpp
@@ -50,7 +50,7 @@ float DistanceBetween(const Rect &r1, const Rect &r2) {
 	);
 	int inner_width = std::max(0, rect_outer.GetWidth() - r1.GetWidth() - r2.GetWidth());
 	int inner_height = std::max(0, rect_outer.GetHeight() - r1.GetHeight() - r2.GetHeight());
-	return std::sqrt((inner_width ^ 2) + (inner_height ^ 2));
+	return static_cast<float>(std::sqrt((inner_width ^ 2) + (inner_height ^ 2)));
 }
 
 Size ProportionalStretch(int dest_w, int dest_h, int item_w, int item_h) {
diff --git a/engines/ags/shared/util/string.cpp b/engines/ags/shared/util/string.cpp
index d79c28e00b..93c1d6ec70 100644
--- a/engines/ags/shared/util/string.cpp
+++ b/engines/ags/shared/util/string.cpp
@@ -432,8 +432,8 @@ void String::Append(const char *cstr, size_t len) {
 		return;
 	// Test for null-terminator in the range
 	const char *ptr = cstr;
-	for (; *ptr && (size_t)(ptr - cstr) < len; ++ptr);
-	if ((size_t)(ptr - cstr) < len)
+	for (; *ptr && (static_cast<size_t>(ptr - cstr) < len); ++ptr);
+	if (static_cast<size_t>(ptr - cstr) < len)
 		len = ptr - cstr;
 	ReserveAndShift(false, len);
 	memcpy(_cstr + _len, cstr, len);


Commit: f5a019d3b4b82e5ebd3113020287e5f3d4a21005
    https://github.com/scummvm/scummvm/commit/f5a019d3b4b82e5ebd3113020287e5f3d4a21005
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:58-07:00

Commit Message:
AGS: corrected GetDisplayDepthForNativeDepth

>From upstream 4c51ca2045b478b63f5e6b35be02fdce3ebf9b18

Changed paths:
    engines/ags/engine/gfx/ali_3d_scummvm.cpp


diff --git a/engines/ags/engine/gfx/ali_3d_scummvm.cpp b/engines/ags/engine/gfx/ali_3d_scummvm.cpp
index 5bdd5c8db4..b7dc3ee8ce 100644
--- a/engines/ags/engine/gfx/ali_3d_scummvm.cpp
+++ b/engines/ags/engine/gfx/ali_3d_scummvm.cpp
@@ -69,10 +69,7 @@ bool ScummVMRendererGraphicsDriver::IsModeSupported(const DisplayMode &mode) {
 }
 
 int ScummVMRendererGraphicsDriver::GetDisplayDepthForNativeDepth(int native_color_depth) const {
-	// TODO: check for device caps to know which depth is supported?
-	if (native_color_depth > 8)
-		return 32;
-	return native_color_depth;
+	return 32;
 }
 
 IGfxModeList *ScummVMRendererGraphicsDriver::GetSupportedModeList(int color_depth) {


Commit: f6e5133edd5f3bec4fddb4727bcbbc086b530fa9
    https://github.com/scummvm/scummvm/commit/f6e5133edd5f3bec4fddb4727bcbbc086b530fa9
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:58-07:00

Commit Message:
AGS: In ci_find_file remove gratuitous chdir() code

>From upstream bc4c4ad1a8374e9890e8c44bf560548700bce311

Changed paths:
    engines/ags/shared/util/misc.cpp


diff --git a/engines/ags/shared/util/misc.cpp b/engines/ags/shared/util/misc.cpp
index 90016ec2cc..a1e1a97ec2 100644
--- a/engines/ags/shared/util/misc.cpp
+++ b/engines/ags/shared/util/misc.cpp
@@ -88,7 +88,6 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 	struct stat   statbuf;
 	struct dirent *entry = nullptr;
 	DIR *rough = nullptr;
-	DIR *prevdir = nullptr;
 	char *diamond = nullptr;
 	char *directory = nullptr;
 	char *filename = nullptr;
@@ -158,19 +157,9 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 		filename[match_len] = '\0';
 	}
 
-	if ((prevdir = opendir(".")) == nullptr) {
-		fprintf(stderr, "ci_find_file: cannot open current working directory\n");
-		goto out;
-	}
-
-	if (chdir(directory) == -1) {
-		fprintf(stderr, "ci_find_file: cannot change to directory: %s\n", directory);
-		goto out_pd;
-	}
-
 	if ((rough = opendir(directory)) == nullptr) {
 		fprintf(stderr, "ci_find_file: cannot open directory: %s\n", directory);
-		goto out_pd;
+		goto out;
 	}
 
 	while ((entry = readdir(rough)) != nullptr) {
@@ -188,10 +177,6 @@ char *ci_find_file(const char *dir_name, const char *file_name) {
 	}
 	closedir(rough);
 
-out_pd:;
-	fchdir(dirfd(prevdir));
-	closedir(prevdir);
-
 out:;
 	if (directory) free(directory);
 	if (filename) free(filename);


Commit: d9bbf9775cda4442e15b038a5573d5af3c6696d9
    https://github.com/scummvm/scummvm/commit/d9bbf9775cda4442e15b038a5573d5af3c6696d9
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2021-07-10T18:56:59-07:00

Commit Message:
AGS: Rewrote ci_find_file() into File::FindFileCI()

>From upstream 4e2df949ff36bbb6d08e402586dc8c2bb02ecad9

Changed paths:
    engines/ags/ags.h
    engines/ags/engine/ac/file.cpp
    engines/ags/engine/ac/translation.cpp
    engines/ags/engine/main/engine.cpp
    engines/ags/engine/main/main_header.h
    engines/ags/engine/script/cc_instance.cpp
    engines/ags/shared/core/asset_manager.cpp
    engines/ags/shared/util/compress.cpp
    engines/ags/shared/util/file.cpp
    engines/ags/shared/util/file.h
    engines/ags/shared/util/string_utils.cpp
    engines/ags/shared/util/string_utils.h


diff --git a/engines/ags/ags.h b/engines/ags/ags.h
index 8be4ede7fd..7cfd80ecf5 100644
--- a/engines/ags/ags.h
+++ b/engines/ags/ags.h
@@ -49,8 +49,8 @@ namespace AGS {
  * @brief Engine to run Adventure Game Studio games.
  */
 
-/* Synced up to upstream: AGS 3.5.1.8
- * 811f6c126fcff1cc5a0db8b6e0a1b9fca2550235
+/* Synced up to upstream:
+ * 4e2df949ff36bbb6d08e402586dc8c2bb02ecad9
  */
 #define SCREEN_WIDTH 320
 #define SCREEN_HEIGHT 200
diff --git a/engines/ags/engine/ac/file.cpp b/engines/ags/engine/ac/file.cpp
index acbe86d028..7f667bd5bd 100644
--- a/engines/ags/engine/ac/file.cpp
+++ b/engines/ags/engine/ac/file.cpp
@@ -34,7 +34,6 @@
 #include "ags/engine/ac/string.h"
 #include "ags/engine/debugging/debug_log.h"
 #include "ags/engine/debugging/debugger.h"
-#include "ags/shared/util/misc.h"
 #include "ags/engine/platform/base/ags_platform_driver.h"
 #include "ags/shared/util/stream.h"
 #include "ags/shared/core/asset_manager.h"
@@ -477,12 +476,12 @@ bool DoesAssetExistInLib(const AssetPath &path) {
 }
 
 String find_assetlib(const String &filename) {
-	String libname = cbuf_to_string_and_free(ci_find_file(_GP(ResPaths).DataDir.GetCStr(), filename.GetCStr()));
+	String libname = File::FindFileCI(_GP(ResPaths).DataDir, filename);
 	if (AssetManager::IsDataFile(libname))
 		return libname;
 	if (Path::ComparePaths(_GP(ResPaths).DataDir, _GP(ResPaths).DataDir2) != 0) {
 		// Hack for running in Debugger
-		libname = cbuf_to_string_and_free(ci_find_file(_GP(ResPaths).DataDir2.GetCStr(), filename.GetCStr()));
+		libname = File::FindFileCI(_GP(ResPaths).DataDir2, filename);
 		if (AssetManager::IsDataFile(libname))
 			return libname;
 	}
diff --git a/engines/ags/engine/ac/translation.cpp b/engines/ags/engine/ac/translation.cpp
index 7cd27a2bc8..26e74c3bb5 100644
--- a/engines/ags/engine/ac/translation.cpp
+++ b/engines/ags/engine/ac/translation.cpp
@@ -32,7 +32,6 @@
 #include "ags/shared/ac/words_dictionary.h"
 #include "ags/shared/debugging/out.h"
 #include "ags/shared/game/tra_file.h"
-#include "ags/shared/util/misc.h"
 #include "ags/shared/util/stream.h"
 #include "ags/shared/core/asset_manager.h"
 #include "ags/globals.h"
diff --git a/engines/ags/engine/main/engine.cpp b/engines/ags/engine/main/engine.cpp
index 0130c3c9ca..890efcc5cd 100644
--- a/engines/ags/engine/main/engine.cpp
+++ b/engines/ags/engine/main/engine.cpp
@@ -78,10 +78,7 @@
 #include "ags/engine/platform/base/ags_platform_driver.h"
 #include "ags/shared/util/directory.h"
 #include "ags/shared/util/error.h"
-#include "ags/shared/util/misc.h"
 #include "ags/shared/util/path.h"
-//#include "ags/engine/media/audio/audio_core.h"
-//#include "ags/engine/platform/util/pe.h"
 #include "ags/ags.h"
 #include "ags/globals.h"
 
diff --git a/engines/ags/engine/main/main_header.h b/engines/ags/engine/main/main_header.h
index c262d725d6..074fcb91f0 100644
--- a/engines/ags/engine/main/main_header.h
+++ b/engines/ags/engine/main/main_header.h
@@ -33,7 +33,6 @@
 #include "ags/shared/util/string_utils.h"
 #include "ags/engine/device/mouse_w32.h"
 #include "ags/engine/ac/route_finder.h"
-#include "ags/shared/util/misc.h"
 #include "ags/shared/script/cc_error.h"
 
 // include last since we affect windows includes
diff --git a/engines/ags/engine/script/cc_instance.cpp b/engines/ags/engine/script/cc_instance.cpp
index 7cd8573092..29ef35b16c 100644
--- a/engines/ags/engine/script/cc_instance.cpp
+++ b/engines/ags/engine/script/cc_instance.cpp
@@ -20,8 +20,6 @@
  *
  */
 
-//include <cstdio>
-//include <string.h>
 #include "ags/shared/ac/common.h"
 #include "ags/engine/ac/dynobj/cc_dynamic_array.h"
 #include "ags/engine/ac/dynobj/managed_object_pool.h"
@@ -35,8 +33,8 @@
 #include "ags/engine/script/script_runtime.h"
 #include "ags/engine/script/system_imports.h"
 #include "ags/shared/util/bbop.h"
+#include "ags/shared/util/file.h"
 #include "ags/shared/util/stream.h"
-#include "ags/shared/util/misc.h"
 #include "ags/shared/util/text_stream_writer.h"
 #include "ags/engine/ac/dynobj/script_string.h"
 #include "ags/engine/ac/dynobj/script_user_object.h"
@@ -1247,7 +1245,7 @@ void ccInstance::DumpInstruction(const ScriptOperation &op) {
 		return;
 	}
 
-	Stream *data_s = ci_fopen("script.log", kFile_Create, kFile_Write);
+	Stream *data_s = File::OpenFileCI("script.log", kFile_Create, kFile_Write);
 	TextStreamWriter writer(data_s);
 	writer.WriteFormat("Line %3d, IP:%8d (SP:%p) ", line_num, pc, registers[SREG_SP].RValue);
 
diff --git a/engines/ags/shared/core/asset_manager.cpp b/engines/ags/shared/core/asset_manager.cpp
index 3df043d1e4..7f3a874326 100644
--- a/engines/ags/shared/core/asset_manager.cpp
+++ b/engines/ags/shared/core/asset_manager.cpp
@@ -25,7 +25,6 @@
 #include "ags/lib/std/utility.h"
 #include "ags/shared/core/platform.h"
 #include "ags/shared/core/asset_manager.h"
-#include "ags/shared/util/misc.h" // ci_fopen
 #include "ags/shared/util/multi_file_lib.h"
 #include "ags/shared/util/path.h"
 #include "ags/shared/util/string_utils.h" // cbuf_to_string_and_free
@@ -61,7 +60,7 @@ bool AssetManager::LibsByPriority::operator()(const AssetLibInfo *lib1, const As
 
 
 /* static */ bool AssetManager::IsDataFile(const String &data_file) {
-	Stream *in = ci_fopen(data_file.GetCStr(), Shared::kFile_Open, Shared::kFile_Read);
+	Stream *in = File::OpenFileCI(data_file.GetCStr(), Shared::kFile_Open, Shared::kFile_Read);
 	if (in) {
 		MFLUtil::MFLError err = MFLUtil::TestIsMFL(in, true);
 		delete in;
@@ -71,7 +70,7 @@ bool AssetManager::LibsByPriority::operator()(const AssetLibInfo *lib1, const As
 }
 
 /* static */ AssetError AssetManager::ReadDataFileTOC(const String &data_file, AssetLibInfo &lib) {
-	Stream *in = ci_fopen(data_file.GetCStr(), Shared::kFile_Open, Shared::kFile_Read);
+	Stream *in = File::OpenFileCI(data_file.GetCStr(), Shared::kFile_Open, Shared::kFile_Read);
 	if (in) {
 		MFLUtil::MFLError err = MFLUtil::ReadHeader(lib, in);
 		delete in;
@@ -178,7 +177,7 @@ AssetError AssetManager::RegisterAssetLib(const String &path, AssetLibEx *&out_l
 	}
 	// ...else try open a data library
 	else {
-		Stream *in = ci_fopen(path.GetCStr(), Shared::kFile_Open, Shared::kFile_Read);
+		Stream *in = File::OpenFileCI(path.GetCStr(), Shared::kFile_Open, Shared::kFile_Read);
 		if (!in)
 			return kAssetErrNoLibFile; // can't be opened, return error code
 
@@ -234,7 +233,7 @@ bool AssetManager::GetAssetFromLib(const AssetLibInfo *lib, const String &asset_
 	if (asset == nullptr)
 		return false;
 
-	String libfile = cbuf_to_string_and_free(ci_find_file(lib->BaseDir.GetCStr(), lib->LibFileNames[asset->LibUid].GetCStr()));
+	String libfile = File::FindFileCI(lib->BaseDir, lib->LibFileNames[asset->LibUid]);
 	if (libfile.IsEmpty())
 		return false;
 	if (loc) {
@@ -247,7 +246,7 @@ bool AssetManager::GetAssetFromLib(const AssetLibInfo *lib, const String &asset_
 
 bool AssetManager::GetAssetFromDir(const AssetLibInfo *lib, const String &file_name,
                                    AssetLocation *loc, FileOpenMode open_mode, FileWorkMode work_mode) const {
-	String found_file = cbuf_to_string_and_free(ci_find_file(lib->BaseDir.GetCStr(), file_name.GetCStr()));
+	String found_file = File::FindFileCI(lib->BaseDir, file_name);
 	if (found_file.IsEmpty() || !Path::IsFile(found_file))
 		return false; // not found, or not a file
 
diff --git a/engines/ags/shared/util/compress.cpp b/engines/ags/shared/util/compress.cpp
index a58d2dc0b0..d65320b9cf 100644
--- a/engines/ags/shared/util/compress.cpp
+++ b/engines/ags/shared/util/compress.cpp
@@ -28,8 +28,8 @@
 #include "ags/shared/ac/common.h"   // quit, update_polled_stuff
 #include "ags/shared/gfx/bitmap.h"
 #include "ags/shared/util/compress.h"
+#include "ags/shared/util/file.h"
 #include "ags/shared/util/lzw.h"
-#include "ags/shared/util/misc.h"
 #include "ags/shared/util/stream.h"
 #include "ags/globals.h"
 #if AGS_PLATFORM_ENDIAN_BIG
@@ -326,14 +326,14 @@ const char *lztempfnm = "~aclzw.tmp";
 
 void save_lzw(Stream *out, const Bitmap *bmpp, const RGB *pall) {
 	// First write original bitmap into temporary file
-	Stream *lz_temp_s = ci_fopen(lztempfnm, kFile_CreateAlways, kFile_Write);
+	Stream *lz_temp_s = File::OpenFileCI(lztempfnm, kFile_CreateAlways, kFile_Write);
 	lz_temp_s->WriteInt32(bmpp->GetWidth() * bmpp->GetBPP());
 	lz_temp_s->WriteInt32(bmpp->GetHeight());
 	lz_temp_s->WriteArray(bmpp->GetData(), bmpp->GetLineLength(), bmpp->GetHeight());
 	delete lz_temp_s;
 
 	// Now open same file for reading, and begin writing compressed data into required output stream
-	lz_temp_s = ci_fopen(lztempfnm);
+	lz_temp_s = File::OpenFileCI(lztempfnm);
 	soff_t temp_sz = lz_temp_s->GetLength();
 	out->WriteArray(&pall[0], sizeof(RGB), 256);
 	out->WriteInt32(temp_sz);
diff --git a/engines/ags/shared/util/file.cpp b/engines/ags/shared/util/file.cpp
index b6cf4598e2..15189f208c 100644
--- a/engines/ags/shared/util/file.cpp
+++ b/engines/ags/shared/util/file.cpp
@@ -21,11 +21,12 @@
  */
 
 #include "ags/shared/core/platform.h"
-#include "ags/shared/util/stdio_compat.h"
-#include "ags/shared/util/file_stream.h"
 #include "ags/shared/util/buffered_stream.h"
-#include "ags/shared/util/file.h"
 #include "ags/shared/util/directory.h"
+#include "ags/shared/util/file.h"
+#include "ags/shared/util/file_stream.h"
+#include "ags/shared/util/path.h"
+#include "ags/shared/util/stdio_compat.h"
 #include "common/file.h"
 #include "common/savefile.h"
 #include "common/system.h"
@@ -146,6 +147,96 @@ Stream *File::OpenFile(const String &filename, FileOpenMode open_mode, FileWorkM
 	return fs;
 }
 
+String File::FindFileCI(const String &dir_name, const String &file_name) {
+#if !defined (AGS_CASE_SENSITIVE_FILESYSTEM)
+	// Simply concat dir and filename paths
+	return Path::ConcatPaths(dir_name, file_name);
+#else
+	// Case insensitive file find - on case sensitive filesystems
+	//
+	// TODO: still not covered: a situation when the file_name contains
+	// nested path -and- the case of at least one path parts does not match
+	// (with all matching case the file will be found by an early check).
+	//
+	struct stat   statbuf;
+	struct dirent *entry = nullptr;
+
+	if (dir_name.IsEmpty() && file_name.IsEmpty())
+		return nullptr;
+
+	String directory;
+	String filename;
+	String buf;
+
+	if (!dir_name.IsEmpty()) {
+		directory = dir_name;
+		Path::FixupPath(directory);
+	}
+	if (!file_name.IsEmpty()) {
+		filename = file_name;
+		Path::FixupPath(filename);
+	}
+
+	if (!filename.IsEmpty()) {
+		// TODO: move this case to ConcatPaths too?
+		if (directory.IsEmpty() && filename[0] == '/')
+			buf = filename;
+		else
+			buf = Path::ConcatPaths(directory.IsEmpty() ? "." : directory, filename);
+
+		if (lstat(buf.GetCStr(), &statbuf) == 0 &&
+			(S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode))) {
+			return buf;
+		}
+	}
+
+	if (directory.IsEmpty()) {
+		String match = Path::GetFilename(filename);
+		if (match.IsEmpty())
+			return nullptr;
+		directory = Path::GetParent(filename);
+		filename = match;
+	}
+
+	DIR *rough = nullptr;
+	if ((rough = opendir(directory.GetCStr())) == nullptr) {
+		fprintf(stderr, "ci_find_file: cannot open directory: %s\n", directory.GetCStr());
+		return nullptr;
+	}
+
+	String diamond;
+	while ((entry = readdir(rough)) != nullptr) {
+		if (strcasecmp(filename.GetCStr(), entry->d_name) == 0) {
+			if (lstat(entry->d_name, &statbuf) == 0 &&
+				(S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode))) {
+#if AGS_PLATFORM_DEBUG
+				fprintf(stderr, "ci_find_file: Looked for %s in rough %s, found diamond %s.\n",
+					filename.GetCStr(), directory.GetCStr(), entry->d_name);
+#endif // AGS_PLATFORM_DEBUG
+				diamond = Path::ConcatPaths(directory, entry->d_name);
+				break;
+			}
+		}
+	}
+	closedir(rough);
+	return diamond;
+#endif
+}
+
+Stream *File::OpenFileCI(const String &file_name, FileOpenMode open_mode, FileWorkMode work_mode) {
+#if !defined (AGS_CASE_SENSITIVE_FILESYSTEM)
+	return File::OpenFile(file_name, open_mode, work_mode);
+#else
+	String fullpath = FindFileCI(nullptr, file_name);
+	if (!fullpath.IsEmpty())
+		return File::OpenFile(fullpath, open_mode, work_mode);
+	// If the file was not found, and it's Create mode, then open new file
+	if (open_mode != kFile_Open)
+		return File::OpenFile(file_name, open_mode, work_mode);
+	return nullptr;
+#endif
+}
+
 } // namespace Shared
 } // namespace AGS
 } // namespace AGS3
diff --git a/engines/ags/shared/util/file.h b/engines/ags/shared/util/file.h
index e858b75f78..e2442f96ce 100644
--- a/engines/ags/shared/util/file.h
+++ b/engines/ags/shared/util/file.h
@@ -82,6 +82,14 @@ inline Stream *OpenFileRead(const String &filename) {
 inline Stream *OpenFileWrite(const String &filename) {
 	return OpenFile(filename, kFile_Create, kFile_Write);
 }
+
+// Case insensitive find file
+String FindFileCI(const String &dir_name, const String &file_name);
+// Case insensitive file open: looks up for the file using FindFileCI
+Stream *OpenFileCI(const String &file_name,
+	FileOpenMode open_mode = kFile_Open,
+	FileWorkMode work_mode = kFile_Read);
+
 } // namespace File
 
 } // namespace Shared
diff --git a/engines/ags/shared/util/string_utils.cpp b/engines/ags/shared/util/string_utils.cpp
index 134a1845cf..e304a1ed4a 100644
--- a/engines/ags/shared/util/string_utils.cpp
+++ b/engines/ags/shared/util/string_utils.cpp
@@ -30,13 +30,6 @@ namespace AGS3 {
 
 using namespace AGS::Shared;
 
-String cbuf_to_string_and_free(char *char_buf) {
-	String s = char_buf;
-	free(char_buf);
-	return s;
-}
-
-
 namespace AGS {
 namespace Shared {
 
diff --git a/engines/ags/shared/util/string_utils.h b/engines/ags/shared/util/string_utils.h
index 6980aac684..f90d88bc5c 100644
--- a/engines/ags/shared/util/string_utils.h
+++ b/engines/ags/shared/util/string_utils.h
@@ -37,10 +37,6 @@ using namespace AGS; // FIXME later
 
 //=============================================================================
 
-// Converts char* to string and frees original malloc-ed array;
-// This is used when we get a malloc'd char array from some utility function.
-Shared::String cbuf_to_string_and_free(char *char_buf);
-
 namespace AGS {
 namespace Shared {
 namespace StrUtil {