Make replace parameter of RenameFile explicit

Also replace a few rename calls with RenameFile calls. Old code doesn't expect rename to overwrite existing files without question, when it in fact can.

src/client/Client.cpp

@@ -1176,7 +1176,7 @@ String Client::DoMigration(ByteString fromDir, ByteString toDir)
 			std::string to = toDir + directory + "/" + item;
 			if (!Platform::FileExists(to))
 			{
-				if (rename(from.c_str(), to.c_str()))
+				if (Platform::RenameFile(from, to, false))
 				{
 					failedCount++;
 					logFile << "failed to move " << from << " to " << to << std::endl;
@@ -1206,7 +1206,7 @@ String Client::DoMigration(ByteString fromDir, ByteString toDir)
 		ByteString to = toDir + filename;
 		if (!Platform::FileExists(to))
 		{
-			if (rename(from.c_str(), to.c_str()))
+			if (Platform::RenameFile(from, to, false))
 			{
 				logFile << "failed to move " << from << " to " << to << std::endl;
 				result << "\n\br" << filename.FromUtf8() << " migration failed\x0E";

src/common/platform/Platform.h

@@ -21,7 +21,7 @@ namespace Platform
 	 * @return true on success
 	 */
 	bool RemoveFile(ByteString filename);
-	bool RenameFile(ByteString filename, ByteString newFilename, bool replace = false);
+	bool RenameFile(ByteString filename, ByteString newFilename, bool replace);
 
 	/**
 	 * @return true on success

src/common/platform/Posix.cpp

@@ -89,6 +89,13 @@ bool RemoveFile(ByteString filename)
 
 bool RenameFile(ByteString filename, ByteString newFilename, bool replace)
 {
+	// TODO: Make atomic :( Could use renameat2 with RENAME_NOREPLACE on linux and
+	// renamex_np with RENAME_EXCL on darwin, but both require filesystem support;
+	// I don't think it's worth it for now. -- LBPHacker
+	if (!replace && FileExists(newFilename))
+	{
+		return false;
+	}
 	return rename(filename.c_str(), newFilename.c_str()) == 0;
 }
 
@@ -154,12 +161,12 @@ void DoRestart()
 
 bool UpdateStart(const std::vector<char> &data)
 {
-	ByteString exeName = Platform::ExecutableName(), updName;
+	ByteString exeName = Platform::ExecutableName();
 
 	if (!exeName.length())
 		return false;
 
-	updName = exeName + "-update";
+	auto updName = exeName + "-update";
 
 	if (!WriteFile(data, updName))
 	{
@@ -173,7 +180,7 @@ bool UpdateStart(const std::vector<char> &data)
 		return false;
 	}
 
-	if (!RenameFile(updName, exeName))
+	if (!RenameFile(updName, exeName, true))
 	{
 		RemoveFile(updName);
 		return false;

src/common/platform/Windows.cpp

@@ -304,7 +304,7 @@ bool UpdateStart(const std::vector<char> &data)
 		updName = exeName.substr(0, exeName.length() - 4);
 	updName = updName + "_upd.exe";
 
-	if (!RenameFile(exeName, updName))
+	if (!RenameFile(exeName, updName, false))
 		return false;
 
 	if (!WriteFile(data, exeName))

src/graphics/Renderer.cpp

@@ -10,7 +10,7 @@
 #include <cmath>
 
 constexpr auto VIDXRES = WINDOWW;
-constexpr auto VIDYRES = WINDOWH;
+// constexpr auto VIDYRES = WINDOWH; // not actually used anywhere
 
 VideoBuffer * Renderer::WallIcon(int wallID, int width, int height)
 {

src/gui/filebrowser/FileBrowserActivity.cpp

@@ -151,8 +151,7 @@ void FileBrowserActivity::RenameSave(SaveFile * file)
 	if (newName.length())
 	{
 		newName = ByteString::Build(directory, PATH_SEP_CHAR, newName, ".cps");
-		int ret = rename(file->GetName().c_str(), newName.c_str());
+		if (!Platform::RenameFile(file->GetName(), newName, false))
-		if (ret)
 			ErrorMessage::Blocking("Error", "Could not rename file");
 		else
 			loadDirectory(directory, "");

src/lua/LuaScriptInterface.cpp

@@ -4050,7 +4050,8 @@ int LuaScriptInterface::fileSystem_move(lua_State * l)
 {
 	auto filename = tpt_lua_checkByteString(l, 1);
 	auto newFilename = tpt_lua_checkByteString(l, 2);
-	lua_pushboolean(l, Platform::RenameFile(filename, newFilename));
+	bool replace = lua_toboolean(l, 3);
+	lua_pushboolean(l, Platform::RenameFile(filename, newFilename, replace));
 	return 1;
 }