Codechange: replace crashlog filenames with std::string in Crashlog

src/crashlog.cpp

@@ -315,20 +315,18 @@ void CrashLog::LogRecentNews(std::back_insert_iterator<std::string> &output_iter
 
 /**
  * Create a timestamped filename.
- * @param filename      The begin where to write at.
- * @param filename_last The last position in the buffer to write to.
  * @param ext           The extension for the filename.
  * @param with_dir      Whether to prepend the filename with the personal directory.
- * @return the number of added characters.
+ * @return The filename
  */
-int CrashLog::CreateFileName(char *filename, const char *filename_last, const char *ext, bool with_dir) const
+std::string CrashLog::CreateFileName(const char *ext, bool with_dir) const
 {
 	static std::string crashname;
 
 	if (crashname.empty()) {
 		crashname = fmt::format("crash{:%Y%m%d%H%M%S}", fmt::gmtime(time(nullptr)));
 	}
-	return seprintf(filename, filename_last, "%s%s%s", with_dir ? _personal_dir.c_str() : "", crashname.c_str(), ext);
+	return fmt::format("{}{}{}", with_dir ? _personal_dir : std::string{}, crashname, ext);
 }
 
 /**
@@ -361,28 +359,24 @@ void CrashLog::FillCrashLog(std::back_insert_iterator<std::string> &output_itera
 
 /**
  * Write the crash log to a file.
- * @note On success the filename will be filled with the full path of the
+ * @note The filename will be written to \c crashlog_filename.
- *       crash log file. Make sure filename is at least \c MAX_PATH big.
- * @param buffer The begin of the buffer to write to the disk.
- * @param filename      Output for the filename of the written file.
- * @param filename_last The last position in the filename buffer.
  * @return true when the crash log was successfully written.
  */
-bool CrashLog::WriteCrashLog(const char *buffer, char *filename, const char *filename_last) const
+bool CrashLog::WriteCrashLog()
 {
-	this->CreateFileName(filename, filename_last, ".log");
+	this->crashlog_filename = this->CreateFileName(".log");
 
-	FILE *file = FioFOpenFile(filename, "w", NO_DIRECTORY);
+	FILE *file = FioFOpenFile(this->crashlog_filename, "w", NO_DIRECTORY);
 	if (file == nullptr) return false;
 
-	size_t len = strlen(buffer);
+	size_t len = this->crashlog.size();
-	size_t written = fwrite(buffer, 1, len, file);
+	size_t written = fwrite(this->crashlog.data(), 1, len, file);
 
 	FioFCloseFile(file);
 	return len == written;
 }
 
-/* virtual */ int CrashLog::WriteCrashDump(char *filename, const char *filename_last) const
+/* virtual */ int CrashLog::WriteCrashDump()
 {
 	/* Stub implementation; not all OSes support this. */
 	return 0;
@@ -390,13 +384,10 @@ bool CrashLog::WriteCrashLog(const char *buffer, char *filename, const char *fil
 
 /**
  * Write the (crash) savegame to a file.
- * @note On success the filename will be filled with the full path of the
+ * @note The filename will be written to \c savegame_filename.
- *       crash save file. Make sure filename is at least \c MAX_PATH big.
- * @param filename      Output for the filename of the written file.
- * @param filename_last The last position in the filename buffer.
  * @return true when the crash save was successfully made.
  */
-bool CrashLog::WriteSavegame(char *filename, const char *filename_last) const
+bool CrashLog::WriteSavegame()
 {
 	/* If the map doesn't exist, saving will fail too. If the map got
 	 * initialised, there is a big chance the rest is initialised too. */
@@ -405,10 +396,10 @@ bool CrashLog::WriteSavegame(char *filename, const char *filename_last) const
 	try {
 		_gamelog.Emergency();
 
-		this->CreateFileName(filename, filename_last, ".sav");
+		this->savegame_filename = this->CreateFileName(".sav");
 
 		/* Don't do a threaded saveload. */
-		return SaveOrLoad(filename, SLO_SAVE, DFT_GAME_FILE, NO_DIRECTORY, false) == SL_OK;
+		return SaveOrLoad(this->savegame_filename, SLO_SAVE, DFT_GAME_FILE, NO_DIRECTORY, false) == SL_OK;
 	} catch (...) {
 		return false;
 	}
@@ -416,21 +407,17 @@ bool CrashLog::WriteSavegame(char *filename, const char *filename_last) const
 
 /**
  * Write the (crash) screenshot to a file.
- * @note On success the filename will be filled with the full path of the
+ * @note The filename will be written to \c screenshot_filename.
- *       screenshot. Make sure filename is at least \c MAX_PATH big.
+ * @return std::nullopt when the crash screenshot could not be made, otherwise the filename.
- * @param filename      Output for the filename of the written file.
- * @param filename_last The last position in the filename buffer.
- * @return true when the crash screenshot was successfully made.
  */
-bool CrashLog::WriteScreenshot(char *filename, const char *filename_last) const
+bool CrashLog::WriteScreenshot()
 {
 	/* Don't draw when we have invalid screen size */
 	if (_screen.width < 1 || _screen.height < 1 || _screen.dst_ptr == nullptr) return false;
 
-	this->CreateFileName(filename, filename_last, "", false);
+	std::string filename = this->CreateFileName("", false);
 	bool res = MakeScreenshot(SC_CRASHLOG, filename);
-	filename[0] = '\0';
+	if (res) this->screenshot_filename = _full_screenshot_name;
-	if (res) strecpy(filename, _full_screenshot_name, filename_last);
 	return res;
 }
 
@@ -447,55 +434,53 @@ void CrashLog::SendSurvey() const
  * information like paths to the console.
  * @return true when everything is made successfully.
  */
-bool CrashLog::MakeCrashLog() const
+bool CrashLog::MakeCrashLog()
 {
 	/* Don't keep looping logging crashes. */
 	static bool crashlogged = false;
 	if (crashlogged) return false;
 	crashlogged = true;
 
-	char filename[MAX_PATH];
+	crashlog.reserve(65536);
-	std::string buffer;
+	auto output_iterator = std::back_inserter(crashlog);
-	buffer.reserve(65536);
-	auto output_iterator = std::back_inserter(buffer);
 	bool ret = true;
 
 	fmt::print("Crash encountered, generating crash log...\n");
 	this->FillCrashLog(output_iterator);
-	fmt::print("{}\n", buffer);
+	fmt::print("{}\n", crashlog);
 	fmt::print("Crash log generated.\n\n");
 
 	fmt::print("Writing crash log to disk...\n");
-	bool bret = this->WriteCrashLog(buffer.c_str(), filename, lastof(filename));
+	bool bret = this->WriteCrashLog();
 	if (bret) {
-		fmt::print("Crash log written to {}. Please add this file to any bug reports.\n\n", filename);
+		fmt::print("Crash log written to {}. Please add this file to any bug reports.\n\n", this->crashlog_filename);
 	} else {
 		fmt::print("Writing crash log failed. Please attach the output above to any bug reports.\n\n");
 		ret = false;
 	}
 
 	/* Don't mention writing crash dumps because not all platforms support it. */
-	int dret = this->WriteCrashDump(filename, lastof(filename));
+	int dret = this->WriteCrashDump();
 	if (dret < 0) {
 		fmt::print("Writing crash dump failed.\n\n");
 		ret = false;
 	} else if (dret > 0) {
-		fmt::print("Crash dump written to {}. Please add this file to any bug reports.\n\n", filename);
+		fmt::print("Crash dump written to {}. Please add this file to any bug reports.\n\n", this->crashdump_filename);
 	}
 
 	fmt::print("Writing crash savegame...\n");
-	bret = this->WriteSavegame(filename, lastof(filename));
+	bret = this->WriteSavegame();
 	if (bret) {
-		fmt::print("Crash savegame written to {}. Please add this file and the last (auto)save to any bug reports.\n\n", filename);
+		fmt::print("Crash savegame written to {}. Please add this file and the last (auto)save to any bug reports.\n\n", this->savegame_filename);
 	} else {
 		ret = false;
 		fmt::print("Writing crash savegame failed. Please attach the last (auto)save to any bug reports.\n\n");
 	}
 
 	fmt::print("Writing crash screenshot...\n");
-	bret = this->WriteScreenshot(filename, lastof(filename));
+	bret = this->WriteScreenshot();
 	if (bret) {
-		fmt::print("Crash screenshot written to {}. Please add this file to any bug reports.\n\n", filename);
+		fmt::print("Crash screenshot written to {}. Please add this file to any bug reports.\n\n", this->screenshot_filename);
 	} else {
 		ret = false;
 		fmt::print("Writing crash screenshot failed.\n\n");

src/crashlog.h

@@ -65,31 +65,34 @@ protected:
 	void LogGamelog(std::back_insert_iterator<std::string> &output_iterator) const;
 	void LogRecentNews(std::back_insert_iterator<std::string> &output_iterator) const;
 
-	int CreateFileName(char *filename, const char *filename_last, const char *ext, bool with_dir = true) const;
+	std::string CreateFileName(const char *ext, bool with_dir = true) const;
 
 public:
 	/** Stub destructor to silence some compilers. */
 	virtual ~CrashLog() = default;
 
+	std::string crashlog;
+	std::string crashlog_filename;
+	std::string crashdump_filename;
+	std::string savegame_filename;
+	std::string screenshot_filename;
+
 	void FillCrashLog(std::back_insert_iterator<std::string> &output_iterator) const;
-	bool WriteCrashLog(const char *buffer, char *filename, const char *filename_last) const;
+	bool WriteCrashLog();
 
 	/**
 	 * Write the (crash) dump to a file.
-	 * @note On success the filename will be filled with the full path of the
+	 * @note Sets \c crashdump_filename when there is a successful return.
-	 *       crash dump file. Make sure filename is at least \c MAX_PATH big.
-	 * @param filename      Output for the filename of the written file.
-	 * @param filename_last The last position in the filename buffer.
 	 * @return if less than 0, error. If 0 no dump is made, otherwise the dump
 	 *         was successful (not all OSes support dumping files).
 	 */
-	virtual int WriteCrashDump(char *filename, const char *filename_last) const;
+	virtual int WriteCrashDump();
-	bool WriteSavegame(char *filename, const char *filename_last) const;
+	bool WriteSavegame();
-	bool WriteScreenshot(char *filename, const char *filename_last) const;
+	bool WriteScreenshot();
 
 	void SendSurvey() const;
 
-	bool MakeCrashLog() const;
+	bool MakeCrashLog();
 
 	/**
 	 * Initialiser for crash logs; do the appropriate things so crashes are

src/os/macosx/crashlog_osx.cpp

@@ -40,10 +40,6 @@ class CrashLogOSX : public CrashLog {
 	/** Signal that has been thrown. */
 	int signum;
 
-	char filename_log[MAX_PATH];        ///< Path of crash.log
-	char filename_save[MAX_PATH];       ///< Path of crash.sav
-	char filename_screenshot[MAX_PATH]; ///< Path of crash.(png|bmp|pcx)
-
 	void LogOSVersion(std::back_insert_iterator<std::string> &output_iterator) const override
 	{
 		int ver_maj, ver_min, ver_bug;
@@ -149,12 +145,7 @@ public:
 	 * A crash log is always generated by signal.
 	 * @param signum the signal that was caused by the crash.
 	 */
-	CrashLogOSX(int signum) : signum(signum)
+	CrashLogOSX(int signum) : signum(signum) {}
-	{
-		filename_log[0] = '\0';
-		filename_save[0] = '\0';
-		filename_screenshot[0] = '\0';
-	}
 
 	/** Show a dialog with the crash information. */
 	void DisplayCrashDialog() const
@@ -162,14 +153,13 @@ public:
 		static const char crash_title[] =
 			"A serious fault condition occurred in the game. The game will shut down.";
 
-		char message[1024];
+		std::string message = fmt::format(
-		seprintf(message, lastof(message),
 				 "Please send the generated crash information and the last (auto)save to the developers. "
 				 "This will greatly help debugging. The correct place to do this is https://github.com/OpenTTD/OpenTTD/issues.\n\n"
-				 "Generated file(s):\n%s\n%s\n%s",
+				 "Generated file(s):\n{}\n{}\n{}",
-				 this->filename_log, this->filename_save, this->filename_screenshot);
+				 this->crashlog_filename, this->savegame_filename, this->screenshot_filename);
 
-		ShowMacDialog(crash_title, message, "Quit");
+		ShowMacDialog(crash_title, message.c_str(), "Quit");
 	}
 };
 

src/os/windows/crashlog_win.cpp

@@ -39,33 +39,17 @@ class CrashLogWindows : public CrashLog {
 	void LogModules(std::back_insert_iterator<std::string> &output_iterator) const override;
 public:
 #if defined(_MSC_VER)
-	int WriteCrashDump(char *filename, const char *filename_last) const override;
+	int WriteCrashDump() override;
 	void AppendDecodedStacktrace(std::back_insert_iterator<std::string> &output_iterator) const;
 #else
 	void AppendDecodedStacktrace(std::back_insert_iterator<std::string> &output_iterator) const {}
 #endif /* _MSC_VER */
 
-	/** Buffer for the generated crash log */
-	std::string crashlog;
-	/** Buffer for the filename of the crash log */
-	char crashlog_filename[MAX_PATH];
-	/** Buffer for the filename of the crash dump */
-	char crashdump_filename[MAX_PATH];
-	/** Buffer for the filename of the crash screenshot */
-	char screenshot_filename[MAX_PATH];
-
 	/**
 	 * A crash log is always generated when it's generated.
 	 * @param ep the data related to the exception.
 	 */
-	CrashLogWindows(EXCEPTION_POINTERS *ep = nullptr) :
+	CrashLogWindows(EXCEPTION_POINTERS *ep = nullptr) : ep(ep) {}
-		ep(ep)
-	{
-		this->crashlog.reserve(65536);
-		this->crashlog_filename[0] = '\0';
-		this->crashdump_filename[0] = '\0';
-		this->screenshot_filename[0] = '\0';
-	}
 
 	/**
 	 * Points to the current crash log.
@@ -90,7 +74,6 @@ public:
 			os.dwBuildNumber,
 			os.szCSDVersion
 	);
-
 }
 
 /* virtual */ void CrashLogWindows::LogError(std::back_insert_iterator<std::string> &output_iterator, const std::string_view message) const
@@ -466,7 +449,7 @@ void CrashLogWindows::AppendDecodedStacktrace(std::back_insert_iterator<std::str
 	fmt::format_to(output_iterator, "\n*** End of additional info ***\n");
 }
 
-/* virtual */ int CrashLogWindows::WriteCrashDump(char *filename, const char *filename_last) const
+/* virtual */ int CrashLogWindows::WriteCrashDump()
 {
 	int ret = 0;
 	DllLoader dbghelp(L"dbghelp.dll");
@@ -478,8 +461,8 @@ void CrashLogWindows::AppendDecodedStacktrace(std::back_insert_iterator<std::str
 				CONST PMINIDUMP_CALLBACK_INFORMATION);
 		MiniDumpWriteDump_t funcMiniDumpWriteDump = dbghelp.GetProcAddress("MiniDumpWriteDump");
 		if (funcMiniDumpWriteDump != nullptr) {
-			this->CreateFileName(filename, filename_last, ".dmp");
+			this->crashdump_filename = this->CreateFileName(".dmp");
-			HANDLE file  = CreateFile(OTTD2FS(filename).c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, 0, 0);
+			HANDLE file  = CreateFile(OTTD2FS(this->crashdump_filename).c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, 0, 0);
 			HANDLE proc  = GetCurrentProcess();
 			DWORD procid = GetCurrentProcessId();
 			MINIDUMP_EXCEPTION_INFORMATION mdei;
@@ -550,10 +533,10 @@ static LONG WINAPI ExceptionHandler(EXCEPTION_POINTERS *ep)
 	CrashLogWindows::current = log;
 	auto output_iterator = std::back_inserter(log->crashlog);
 	log->FillCrashLog(output_iterator);
-	log->WriteCrashDump(log->crashdump_filename, lastof(log->crashdump_filename));
+	log->WriteCrashDump();
 	log->AppendDecodedStacktrace(output_iterator);
-	log->WriteCrashLog(log->crashlog.c_str(), log->crashlog_filename, lastof(log->crashlog_filename));
+	log->WriteCrashLog();
-	log->WriteScreenshot(log->screenshot_filename, lastof(log->screenshot_filename));
+	log->WriteScreenshot();
 	log->SendSurvey();
 
 	/* Close any possible log files */
@@ -715,9 +698,8 @@ static INT_PTR CALLBACK CrashDialogFunc(HWND wnd, UINT msg, WPARAM wParam, LPARA
 					ExitProcess(2);
 				case 13: // Emergency save
 					wchar_t filenamebuf[MAX_PATH * 2];
-					char filename[MAX_PATH];
+					if (CrashLogWindows::current->WriteSavegame()) {
-					if (CrashLogWindows::current->WriteSavegame(filename, lastof(filename))) {
+						convert_to_fs(CrashLogWindows::current->savegame_filename, filenamebuf, lengthof(filenamebuf));
-						convert_to_fs(filename, filenamebuf, lengthof(filenamebuf));
 						size_t len = lengthof(_save_succeeded) + wcslen(filenamebuf) + 1;
 						static wchar_t text[lengthof(_save_succeeded) + MAX_PATH * 2 + 1];
 						_snwprintf(text, len, _save_succeeded, filenamebuf);