Merge pull request #16675 from Iskustvo/improved_error_detectioin_in_move-to-trash_for_Linux

[X11] Improving error detection in move_to_trash

2 files changed, 38 insertions, 18 deletions

diff --git a/drivers/unix/dir_access_unix.cpp b/drivers/unix/dir_access_unix.cpp
index 4a467293fd..3944431516 100644
--- a/drivers/unix/dir_access_unix.cpp
+++ b/drivers/unix/dir_access_unix.cpp
@@ -361,6 +361,7 @@ Error DirAccessUnix::rename(String p_path, String p_new_path) {
 
 	return ::rename(p_path.utf8().get_data(), p_new_path.utf8().get_data()) == 0 ? OK : FAILED;
 }
+
 Error DirAccessUnix::remove(String p_path) {
 
 	if (p_path.is_rel_path())
diff --git a/platform/x11/os_x11.cpp b/platform/x11/os_x11.cpp
index ceaa5ddb07..2027667af3 100644
--- a/platform/x11/os_x11.cpp
+++ b/platform/x11/os_x11.cpp
@@ -32,6 +32,7 @@
 #include "drivers/gles3/rasterizer_gles3.h"
 #include "errno.h"
 #include "key_mapping_x11.h"
+#include "os/dir_access.h"
 #include "print_string.h"
 #include "servers/visual/visual_server_raster.h"
 #include "servers/visual/visual_server_wrap_mt.h"
@@ -2595,48 +2596,66 @@ static String get_mountpoint(const String &p_path) {
 }
 
 Error OS_X11::move_to_trash(const String &p_path) {
-	String trashcan = "";
+	String trash_can = "";
 	String mnt = get_mountpoint(p_path);
 
+	// If there is a directory "[Mountpoint]/.Trash-[UID]/files", use it as the trash can.
 	if (mnt != "") {
 		String path(mnt + "/.Trash-" + itos(getuid()) + "/files");
 		struct stat s;
 		if (!stat(path.utf8().get_data(), &s)) {
-			trashcan = path;
+			trash_can = path;
 		}
 	}
 
-	if (trashcan == "") {
+	// Otherwise, if ${XDG_DATA_HOME} is defined, use "${XDG_DATA_HOME}/Trash/files" as the trash can.
+	if (trash_can == "") {
 		char *dhome = getenv("XDG_DATA_HOME");
 		if (dhome) {
-			trashcan = String(dhome) + "/Trash/files";
+			trash_can = String(dhome) + "/Trash/files";
 		}
 	}
 
-	if (trashcan == "") {
+	// Otherwise, if ${HOME} is defined, use "${HOME}/.local/share/Trash/files" as the trash can.
+	if (trash_can == "") {
 		char *home = getenv("HOME");
 		if (home) {
-			trashcan = String(home) + "/.local/share/Trash/files";
+			trash_can = String(home) + "/.local/share/Trash/files";
 		}
 	}
 
-	if (trashcan == "") {
-		ERR_PRINTS("move_to_trash: Could not determine trashcan location");
+	// Issue an error if none of the previous locations is appropriate for the trash can.
+	if (trash_can == "") {
+		ERR_PRINTS("move_to_trash: Could not determine the trash can location");
 		return FAILED;
 	}
 
-	List<String> args;
-	args.push_back("-p");
-	args.push_back(trashcan);
-	Error err = execute("mkdir", args, true);
-	if (err == OK) {
-		List<String> args2;
-		args2.push_back(p_path);
-		args2.push_back(trashcan);
-		err = execute("mv", args2, true);
+	// Create needed directories for decided trash can location.
+	DirAccess *dir_access = DirAccess::create(DirAccess::ACCESS_FILESYSTEM);
+	Error err = dir_access->make_dir_recursive(trash_can);
+	memdelete(dir_access);
+
+	// Issue an error if trash can is not created proprely.
+	if (err != OK) {
+		ERR_PRINTS("move_to_trash: Could not create the trash can \"" + trash_can + "\"");
+		return err;
 	}
 
-	return err;
+	// The trash can is successfully created, now move the given resource to it.
+	// Do not use DirAccess:rename() because it can't move files across multiple mountpoints.
+	List<String> mv_args;
+	mv_args.push_back(p_path);
+	mv_args.push_back(trash_can);
+	int retval;
+	err = execute("mv", mv_args, true, NULL, NULL, &retval);
+
+	// Issue an error if "mv" failed to move the given resource to the trash can.
+	if (err != OK || retval != 0) {
+		ERR_PRINTS("move_to_trash: Could not move the resource \"" + p_path + "\" to the trash can \"" + trash_can + "\"");
+		return FAILED;
+	}
+
+	return OK;
 }
 
 OS::LatinKeyboardVariant OS_X11::get_latin_keyboard_variant() const {