Codechange: replace C-style string processing with C++-style for the listing callbacks

src/fios.cpp

@@ -41,7 +41,7 @@ extern bool FiosIsHiddenFile(const struct dirent *ent);
 extern void FiosGetDrives(FileList &file_list);
 
 /* get the name of an oldstyle savegame */
-extern void GetOldSaveGameName(const std::string &file, char *title, const char *last);
+extern std::string GetOldSaveGameName(const std::string &file);
 
 /**
  * Compare two FiosItem's. Used with sort when sorting the file list.
@@ -215,9 +215,7 @@ static std::string FiosMakeFilename(const std::string *path, const char *name, c
 
 /**
  * Make a save game or scenario filename from a name.
- * @param buf Destination buffer for saving the filename.
  * @param name Name of the file.
- * @param last Last element of buffer \a buf.
  * @return The completed filename.
  */
 std::string FiosMakeSavegameName(const char *name)
@@ -251,14 +249,14 @@ bool FiosDelete(const char *name)
 	return unlink(filename.c_str()) == 0;
 }
 
-typedef FiosType fios_getlist_callback_proc(SaveLoadOperation fop, const std::string &filename, const char *ext, char *title, const char *last);
+typedef std::tuple<FiosType, std::string> FiosGetTypeAndNameProc(SaveLoadOperation fop, const std::string &filename, const std::string_view ext);
 
 /**
  * Scanner to scan for a particular type of FIOS file.
  */
 class FiosFileScanner : public FileScanner {
 	SaveLoadOperation fop;   ///< The kind of file we are looking for.
-	fios_getlist_callback_proc *callback_proc; ///< Callback to check whether the file may be added
+	FiosGetTypeAndNameProc *callback_proc; ///< Callback to check whether the file may be added
 	FileList &file_list;     ///< Destination of the found files.
 public:
 	/**
@@ -267,7 +265,7 @@ public:
 	 * @param callback_proc The function that is called where you need to do the filtering.
 	 * @param file_list Destination of the found files.
 	 */
-	FiosFileScanner(SaveLoadOperation fop, fios_getlist_callback_proc *callback_proc, FileList &file_list) :
+	FiosFileScanner(SaveLoadOperation fop, FiosGetTypeAndNameProc *callback_proc, FileList &file_list) :
 			fop(fop), callback_proc(callback_proc), file_list(file_list)
 	{}
 
@@ -286,10 +284,7 @@ bool FiosFileScanner::AddFile(const std::string &filename, size_t basepath_lengt
 	if (sep == std::string::npos) return false;
 	std::string ext = filename.substr(sep);
 
-	char fios_title[64];
+	auto [type, title] = this->callback_proc(this->fop, filename, ext);
-	fios_title[0] = '\0'; // reset the title
-
-	FiosType type = this->callback_proc(this->fop, filename, ext.c_str(), fios_title, lastof(fios_title));
 	if (type == FIOS_TYPE_INVALID) return false;
 
 	for (const auto &fios : file_list) {
@@ -329,12 +324,12 @@ bool FiosFileScanner::AddFile(const std::string &filename, size_t basepath_lengt
 	fios->name = filename;
 
 	/* If the file doesn't have a title, use its filename */
-	const char *t = fios_title;
+	if (title.empty()) {
-	if (StrEmpty(fios_title)) {
 		auto ps = filename.rfind(PATHSEPCHAR);
-		t = filename.c_str() + (ps == std::string::npos ? 0 : ps + 1);
+		fios->title = StrMakeValid(filename.substr((ps == std::string::npos ? 0 : ps + 1)));
-	}
+	} else {
-	fios->title = StrMakeValid(t);
+		fios->title = StrMakeValid(title);
+	};
 
 	return true;
 }
@@ -347,7 +342,7 @@ bool FiosFileScanner::AddFile(const std::string &filename, size_t basepath_lengt
  * @param subdir The directory from where to start (global) searching.
  * @param file_list Destination of the found files.
  */
-static void FiosGetFileList(SaveLoadOperation fop, fios_getlist_callback_proc *callback_proc, Subdirectory subdir, FileList &file_list)
+static void FiosGetFileList(SaveLoadOperation fop, FiosGetTypeAndNameProc *callback_proc, Subdirectory subdir, FileList &file_list)
 {
 	struct stat sb;
 	struct dirent *dirent;
@@ -421,23 +416,20 @@ static void FiosGetFileList(SaveLoadOperation fop, fios_getlist_callback_proc *c
  * Get the title of a file, which (if exists) is stored in a file named
  * the same as the data file but with '.title' added to it.
  * @param file filename to get the title for
- * @param title the title buffer to fill
- * @param last the last element in the title buffer
  * @param subdir the sub directory to search in
+ * @return The file title.
  */
-static void GetFileTitle(const std::string &file, char *title, const char *last, Subdirectory subdir)
+static std::string GetFileTitle(const std::string &file, Subdirectory subdir)
 {
-	std::string buf = file;
+	FILE *f = FioFOpenFile(file + ".title", "r", subdir);
-	buf += ".title";
+	if (f == nullptr) return {};
 
-	FILE *f = FioFOpenFile(buf, "r", subdir);
+	char title[80];
-	if (f == nullptr) return;
+	size_t read = fread(title, 1, lengthof(title), f);
-
-	size_t read = fread(title, 1, last - title, f);
-	assert(title + read <= last);
-	title[read] = '\0';
-	StrMakeValidInPlace(title, last);
 	FioFCloseFile(f);
+
+	assert(read <= lengthof(title));
+	return StrMakeValid({title, read});
 }
 
 /**
@@ -445,13 +437,11 @@ static void GetFileTitle(const std::string &file, char *title, const char *last,
  * @param fop Purpose of collecting the list.
  * @param file Name of the file to check.
  * @param ext A pointer to the extension identifier inside file
- * @param title Buffer if a callback wants to lookup the title of the file; nullptr to skip the lookup
+ * @return a FIOS_TYPE_* type of the found file, FIOS_TYPE_INVALID if not a savegame, and the title of the file (if any).
- * @param last Last available byte in buffer (to prevent buffer overflows); not used when title == nullptr
- * @return a FIOS_TYPE_* type of the found file, FIOS_TYPE_INVALID if not a savegame
  * @see FiosGetFileList
  * @see FiosGetSavegameList
  */
-FiosType FiosGetSavegameListCallback(SaveLoadOperation fop, const std::string &file, const char *ext, char *title, const char *last)
+std::tuple<FiosType, std::string> FiosGetSavegameListCallback(SaveLoadOperation fop, const std::string &file, const std::string_view ext)
 {
 	/* Show savegame files
 	 * .SAV OpenTTD saved game
@@ -460,22 +450,20 @@ FiosType FiosGetSavegameListCallback(SaveLoadOperation fop, const std::string &f
 	 * .SV2 Transport Tycoon Deluxe (Patch) saved 2-player game */
 
 	/* Don't crash if we supply no extension */
-	if (ext == nullptr) return FIOS_TYPE_INVALID;
+	if (ext.empty()) return { FIOS_TYPE_INVALID, {} };
 
 	if (StrEqualsIgnoreCase(ext, ".sav")) {
-		GetFileTitle(file, title, last, SAVE_DIR);
+		return { FIOS_TYPE_FILE, GetFileTitle(file, SAVE_DIR) };
-		return FIOS_TYPE_FILE;
 	}
 
 	if (fop == SLO_LOAD) {
 		if (StrEqualsIgnoreCase(ext, ".ss1") || StrEqualsIgnoreCase(ext, ".sv1") ||
 				StrEqualsIgnoreCase(ext, ".sv2")) {
-			if (title != nullptr) GetOldSaveGameName(file, title, last);
+			return { FIOS_TYPE_OLDFILE, GetOldSaveGameName(file) };
-			return FIOS_TYPE_OLDFILE;
 		}
 	}
 
-	return FIOS_TYPE_INVALID;
+	return { FIOS_TYPE_INVALID, {} };
 }
 
 /**
@@ -500,31 +488,28 @@ void FiosGetSavegameList(SaveLoadOperation fop, FileList &file_list)
  * @param fop Purpose of collecting the list.
  * @param file Name of the file to check.
  * @param ext A pointer to the extension identifier inside file
- * @param title Buffer if a callback wants to lookup the title of the file
+ * @return a FIOS_TYPE_* type of the found file, FIOS_TYPE_INVALID if not a scenario and the title of the file (if any).
- * @param last Last available byte in buffer (to prevent buffer overflows)
- * @return a FIOS_TYPE_* type of the found file, FIOS_TYPE_INVALID if not a scenario
  * @see FiosGetFileList
  * @see FiosGetScenarioList
  */
-static FiosType FiosGetScenarioListCallback(SaveLoadOperation fop, const std::string &file, const char *ext, char *title, const char *last)
+static std::tuple<FiosType, std::string> FiosGetScenarioListCallback(SaveLoadOperation fop, const std::string &file, const std::string_view ext)
 {
 	/* Show scenario files
 	 * .SCN OpenTTD style scenario file
 	 * .SV0 Transport Tycoon Deluxe (Patch) scenario
 	 * .SS0 Transport Tycoon Deluxe preset scenario */
 	if (StrEqualsIgnoreCase(ext, ".scn")) {
-		GetFileTitle(file, title, last, SCENARIO_DIR);
+		return { FIOS_TYPE_SCENARIO, GetFileTitle(file, SCENARIO_DIR) };
-		return FIOS_TYPE_SCENARIO;
+
 	}
 
 	if (fop == SLO_LOAD) {
 		if (StrEqualsIgnoreCase(ext, ".sv0") || StrEqualsIgnoreCase(ext, ".ss0")) {
-			GetOldSaveGameName(file, title, last);
+			return { FIOS_TYPE_OLD_SCENARIO, GetOldSaveGameName(file) };
-			return FIOS_TYPE_OLD_SCENARIO;
 		}
 	}
 
-	return FIOS_TYPE_INVALID;
+	return { FIOS_TYPE_INVALID, {} };
 }
 
 /**
@@ -547,7 +532,7 @@ void FiosGetScenarioList(SaveLoadOperation fop, FileList &file_list)
 	FiosGetFileList(fop, &FiosGetScenarioListCallback, subdir, file_list);
 }
 
-static FiosType FiosGetHeightmapListCallback(SaveLoadOperation fop, const std::string &file, const char *ext, char *title, const char *last)
+static std::tuple<FiosType, std::string> FiosGetHeightmapListCallback(SaveLoadOperation fop, const std::string &file, const std::string_view ext)
 {
 	/* Show heightmap files
 	 * .PNG PNG Based heightmap files
@@ -562,7 +547,7 @@ static FiosType FiosGetHeightmapListCallback(SaveLoadOperation fop, const std::s
 
 	if (StrEqualsIgnoreCase(ext, ".bmp")) type = FIOS_TYPE_BMP;
 
-	if (type == FIOS_TYPE_INVALID) return FIOS_TYPE_INVALID;
+	if (type == FIOS_TYPE_INVALID) return { FIOS_TYPE_INVALID, {} };
 
 	TarFileList::iterator it = _tar_filelist[SCENARIO_DIR].find(file);
 	if (it != _tar_filelist[SCENARIO_DIR].end()) {
@@ -581,12 +566,10 @@ static FiosType FiosGetHeightmapListCallback(SaveLoadOperation fop, const std::s
 			}
 		}
 
-		if (!match) return FIOS_TYPE_INVALID;
+		if (!match) return { FIOS_TYPE_INVALID, {} };
 	}
 
-	GetFileTitle(file, title, last, HEIGHTMAP_DIR);
+	return { type, GetFileTitle(file, HEIGHTMAP_DIR) };
-
-	return type;
 }
 
 /**
@@ -747,9 +730,9 @@ FiosNumberedSaveName::FiosNumberedSaveName(const std::string &prefix) : prefix(p
 	static std::string _prefix; ///< Static as the lambda needs access to it.
 
 	/* Callback for FiosFileScanner. */
-	static fios_getlist_callback_proc *proc = [](SaveLoadOperation fop, const std::string &file, const char *ext, char *title, const char *last) {
+	static FiosGetTypeAndNameProc *proc = [](SaveLoadOperation fop, const std::string &file, const std::string_view ext) {
-		if (StrEqualsIgnoreCase(ext, ".sav") && StrStartsWith(file, _prefix)) return FIOS_TYPE_FILE;
+		if (StrEqualsIgnoreCase(ext, ".sav") && StrStartsWith(file, _prefix)) return std::tuple(FIOS_TYPE_FILE, std::string{});
-		return FIOS_TYPE_INVALID;
+		return std::tuple(FIOS_TYPE_INVALID, std::string{});
 	};
 
 	/* Prefix to check in the callback. */

src/fios.h

@@ -116,7 +116,7 @@ bool FiosDelete(const char *name);
 std::string FiosMakeHeightmapName(const char *name);
 std::string FiosMakeSavegameName(const char *name);
 
-FiosType FiosGetSavegameListCallback(SaveLoadOperation fop, const std::string &file, const char *ext, char *title, const char *last);
+std::tuple<FiosType, std::string> FiosGetSavegameListCallback(SaveLoadOperation fop, const std::string &file, const std::string_view ext);
 
 void ScanScenarios();
 const char *FindScenario(const ContentInfo *ci, bool md5sum);

src/openttd.cpp

@@ -231,7 +231,7 @@ static void ShowHelp()
 #endif
 }
 
-static void WriteSavegameInfo(const char *name)
+static void WriteSavegameInfo(const std::string &name)
 {
 	extern SaveLoadVersion _sl_version;
 	uint32 last_ottd_rev = 0;
@@ -262,7 +262,7 @@ static void WriteSavegameInfo(const char *name)
 	/* ShowInfo put output to stderr, but version information should go
 	 * to stdout; this is the only exception */
 #if !defined(_WIN32)
-	fmt::print("%s\n", message);
+	fmt::print("{}\n", message);
 #else
 	ShowInfoI(message);
 #endif
@@ -588,7 +588,7 @@ int openttd_main(int argc, char *argv[])
 				/* if the file doesn't exist or it is not a valid savegame, let the saveload code show an error */
 				auto t = _file_to_saveload.name.find_last_of('.');
 				if (t != std::string::npos) {
-					FiosType ft = FiosGetSavegameListCallback(SLO_LOAD, _file_to_saveload.name, _file_to_saveload.name.substr(t).c_str(), nullptr, nullptr);
+					auto [ft, _] = FiosGetSavegameListCallback(SLO_LOAD, _file_to_saveload.name, _file_to_saveload.name.substr(t));
 					if (ft != FIOS_TYPE_INVALID) _file_to_saveload.SetMode(ft);
 				}
 
@@ -608,9 +608,7 @@ int openttd_main(int argc, char *argv[])
 				return ret;
 			}
 
-			char title[80];
+			auto [_, title] = FiosGetSavegameListCallback(SLO_LOAD, mgo.opt, strrchr(mgo.opt, '.'));
-			title[0] = '\0';
-			FiosGetSavegameListCallback(SLO_LOAD, mgo.opt, strrchr(mgo.opt, '.'), title, lastof(title));
 
 			_load_check_data.Clear();
 			SaveOrLoadResult res = SaveOrLoad(mgo.opt, SLO_CHECK, DFT_GAME_FILE, SAVE_DIR, false);

src/saveload/oldloader.cpp

@@ -24,6 +24,8 @@
 
 static const int TTO_HEADER_SIZE = 41;
 static const int TTD_HEADER_SIZE = 49;
+/** The size of the checksum in the name/header of the TTD/TTO savegames. */
+static const int HEADER_CHECKSUM_SIZE = 2;
 
 uint32 _bump_assert_value;
 
@@ -204,66 +206,39 @@ static void InitLoading(LoadgameState *ls)
  * @param title title and checksum
  * @param len   the length of the title to read/checksum
  * @return true iff the title is valid
- * @note the title (incl. checksum) has to be at least 41/49 (HEADER_SIZE) bytes long!
  */
 static bool VerifyOldNameChecksum(char *title, uint len)
 {
 	uint16 sum = 0;
-	for (uint i = 0; i < len - 2; i++) {
+	for (uint i = 0; i < len - HEADER_CHECKSUM_SIZE; i++) {
 		sum += title[i];
 		sum = ROL(sum, 1);
 	}
 
 	sum ^= 0xAAAA; // computed checksum
 
-	uint16 sum2 = title[len - 2]; // checksum in file
+	uint16 sum2 = title[len - HEADER_CHECKSUM_SIZE]; // checksum in file
-	SB(sum2, 8, 8, title[len - 1]);
+	SB(sum2, 8, 8, title[len - HEADER_CHECKSUM_SIZE + 1]);
 
 	return sum == sum2;
 }
 
-static inline bool CheckOldSavegameType(FILE *f, char *temp, const char *last, uint len)
+static std::tuple<SavegameType, std::string> DetermineOldSavegameTypeAndName(FILE *f)
 {
-	assert(last - temp + 1 >= (int)len);
+	char buffer[std::max(TTO_HEADER_SIZE, TTD_HEADER_SIZE)];
-
+	if (fread(buffer, 1, lengthof(buffer), f) != lengthof(buffer)) {
-	if (fread(temp, 1, len, f) != len) {
+		return { SGT_INVALID, "(broken) Unable to read file" };
-		temp[0] = '\0'; // if reading failed, make the name empty
-		return false;
 	}
 
-	bool ret = VerifyOldNameChecksum(temp, len);
+	if (VerifyOldNameChecksum(buffer, TTO_HEADER_SIZE)) {
-	temp[len - 2] = '\0'; // name is null-terminated in savegame, but it's better to be sure
+		return { SGT_TTO, "(TTO)" + StrMakeValid({buffer, TTO_HEADER_SIZE - HEADER_CHECKSUM_SIZE}) };
-	StrMakeValidInPlace(temp, last);
-
-	return ret;
 	}
 
-static SavegameType DetermineOldSavegameType(FILE *f, char *title, const char *last)
+	if (VerifyOldNameChecksum(buffer, TTD_HEADER_SIZE)) {
-{
+		return { SGT_TTD, "(TTD)" + StrMakeValid({buffer, TTD_HEADER_SIZE - HEADER_CHECKSUM_SIZE}) };
-	static_assert(TTD_HEADER_SIZE >= TTO_HEADER_SIZE);
-	char temp[TTD_HEADER_SIZE] = "Unknown";
-
-	SavegameType type = SGT_TTO;
-
-	/* Can't fseek to 0 as in tar files that is not correct */
-	long pos = ftell(f);
-	if (pos >= 0 && !CheckOldSavegameType(f, temp, lastof(temp), TTO_HEADER_SIZE)) {
-		type = SGT_TTD;
-		if (fseek(f, pos, SEEK_SET) < 0 || !CheckOldSavegameType(f, temp, lastof(temp), TTD_HEADER_SIZE)) {
-			type = SGT_INVALID;
-		}
 	}
 
-	if (title != nullptr) {
+	return { SGT_INVALID, "(broken) Unknown" };
-		switch (type) {
-			case SGT_TTO: title = strecpy(title, "(TTO) ", last);    break;
-			case SGT_TTD: title = strecpy(title, "(TTD) ", last);    break;
-			default:      title = strecpy(title, "(broken) ", last); break;
-		}
-		strecpy(title, temp, last);
-	}
-
-	return type;
 }
 
 typedef bool LoadOldMainProc(LoadgameState *ls);
@@ -284,7 +259,8 @@ bool LoadOldSaveGame(const std::string &file)
 		return false;
 	}
 
-	SavegameType type = DetermineOldSavegameType(ls.file, nullptr, nullptr);
+	SavegameType type;
+	std::tie(type, std::ignore) = DetermineOldSavegameTypeAndName(ls.file);
 
 	LoadOldMainProc *proc = nullptr;
 
@@ -314,16 +290,13 @@ bool LoadOldSaveGame(const std::string &file)
 	return true;
 }
 
-void GetOldSaveGameName(const std::string &file, char *title, const char *last)
+std::string GetOldSaveGameName(const std::string &file)
 {
 	FILE *f = FioFOpenFile(file, "rb", NO_DIRECTORY);
+	if (f == nullptr) return {};
 
-	if (f == nullptr) {
+	std::string name;
-		*title = '\0';
+	std::tie(std::ignore, name) = DetermineOldSavegameTypeAndName(f);
-		return;
-	}
-
-	DetermineOldSavegameType(f, title, last);
-
 	fclose(f);
+	return name;
 }

src/script/squirrel.cpp

@@ -269,7 +269,7 @@ void Squirrel::PrintFunc(HSQUIRRELVM vm, const std::string &s)
 	/* Check if we have a custom print function */
 	SQPrintFunc *func = ((Squirrel *)sq_getforeignptr(vm))->print_func;
 	if (func == nullptr) {
-		fmt::print("%s", s);
+		fmt::print("{}", s);
 	} else {
 		(*func)(false, s);
 	}