[PATCH v2 0/1] MR6790: ntdll: use posix_spawn() to invoke unmount command instead of system().

[Michael Lelli (＠toadking)]

Fix umounting filesystems mounted on paths with spaces.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57391

--
  v2: ntdll: use posix_spawn() to invoke unmount command instead of system().
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6790

[PATCH v2 1/1] ntdll: use posix_spawn() to invoke unmount command instead of system().

[Michael Lelli]

From: Michael Lelli <toadking(a)toadking.com>

Fix umounting filesystems mounted on paths with spaces.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57391
---
 dlls/ntdll/unix/file.c | 71 +++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 15 deletions(-)

diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
index 44adc4f4626..5ce04507eff 100644
--- a/dlls/ntdll/unix/file.c
+++ b/dlls/ntdll/unix/file.c
@@ -52,6 +52,7 @@
 #include <sys/socket.h>
 #include <sys/time.h>
 #include <sys/ioctl.h>
+#include <sys/wait.h>
 #ifdef HAVE_SYS_ATTR_H
 #include <sys/attr.h>
 #endif
@@ -105,6 +106,13 @@
 #endif
 #include <time.h>
 #include <unistd.h>
+#include <spawn.h>
+#ifdef __APPLE__
+#include <crt_externs.h>
+#define environ (*_NSGetEnviron())
+#else
+extern char **environ;
+#endif
 
 #include "ntstatus.h"
 #define WIN32_NO_STATUS
@@ -3954,6 +3962,49 @@ NTSTATUS get_full_path( const WCHAR *name, const WCHAR *curdir, WCHAR **path )
 }
 
 
+/***********************************************************************
+ *           run_unmount_command
+ *
+ * Run the platform-specific unmount command for the device.
+ */
+static NTSTATUS run_unmount_command( char *mount_point )
+{
+    NTSTATUS status;
+    posix_spawn_file_actions_t actions;
+    pid_t pid;
+#ifdef __APPLE__
+    static char diskutil[] = "diskutil";
+    static char unmount[] = "unmount";
+    char *argv[4] = {diskutil, unmount, mount_point, NULL};
+#else
+    static char umount[] = "umount";
+    char *argv[3] = {umount, mount_point, NULL};
+#endif
+
+    posix_spawn_file_actions_init( &actions );
+
+    posix_spawn_file_actions_addopen( &actions, 0, "/dev/null", O_RDONLY, 0 );
+    posix_spawn_file_actions_addopen( &actions, 1, "/dev/null", O_WRONLY, 0 );
+    posix_spawn_file_actions_addopen( &actions, 2, "/dev/null", O_WRONLY, 0 );
+
+    if (!posix_spawnp( &pid, argv[0], &actions, NULL, argv, environ ))
+    {
+
+        /* wait for command to complete */
+        pid_t wret;
+        do {
+            wret = waitpid( pid, NULL, 0 );
+        } while (wret < 0 && errno == EINTR);
+        status = STATUS_SUCCESS;
+    }
+    else status = STATUS_NO_MEMORY;
+
+    posix_spawn_file_actions_destroy( &actions );
+
+    return status;
+}
+
+
 /***********************************************************************
  *           unmount_device
  *
@@ -3975,23 +4026,13 @@ static NTSTATUS unmount_device( HANDLE handle )
         {
             if ((mount_point = get_device_mount_point( st.st_rdev )))
             {
-#ifdef __APPLE__
-                static const char umount[] = "diskutil unmount >/dev/null 2>&1 ";
-#else
-                static const char umount[] = "umount >/dev/null 2>&1 ";
-#endif
-                char *cmd;
-                if (asprintf( &cmd, "%s%s", umount, mount_point ) != -1)
-                {
-                    system( cmd );
-                    free( cmd );
+                if ((status = run_unmount_command( mount_point ))) return status;
+
 #ifdef linux
-                    /* umount will fail to release the loop device since we still have
-                       a handle to it, so we release it here */
-                    if (major(st.st_rdev) == LOOP_MAJOR) ioctl( unix_fd, 0x4c01 /*LOOP_CLR_FD*/, 0 );
+                /* umount will fail to release the loop device since we still have
+                    a handle to it, so we release it here */
+                if (major(st.st_rdev) == LOOP_MAJOR) ioctl( unix_fd, 0x4c01 /*LOOP_CLR_FD*/, 0 );
 #endif
-                }
-                free( mount_point );
             }
         }
         if (needs_close) close( unix_fd );
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6790

[PATCH v2 1/1] ntdll: use posix_spawn() to invoke unmount command instead of system().

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=149536

Your paranoid android.


=== debian11b (64 bit WoW report) ===

Report validation errors:
quartz:filtergraph crashed (c0000008)
shell32:shelllink crashed (c0000005)