Merge pull request #44514 from madmiraal/split-os-execute

Split OS::execute into two methods

3 files changed, 51 insertions, 43 deletions

diff --git a/platform/windows/export/export.cpp b/platform/windows/export/export.cpp
index 084a5bee1d..222597b3ff 100644
--- a/platform/windows/export/export.cpp
+++ b/platform/windows/export/export.cpp
@@ -173,11 +173,11 @@ void EditorExportPlatformWindows::_rcedit_add_data(const Ref<EditorExportPreset>
 	}
 
 #ifdef WINDOWS_ENABLED
-	OS::get_singleton()->execute(rcedit_path, args, true);
+	OS::get_singleton()->execute(rcedit_path, args);
 #else
 	// On non-Windows we need WINE to run rcedit
 	args.push_front(rcedit_path);
-	OS::get_singleton()->execute(wine_path, args, true);
+	OS::get_singleton()->execute(wine_path, args);
 #endif
 }
 
@@ -314,7 +314,7 @@ Error EditorExportPlatformWindows::_code_sign(const Ref<EditorExportPreset> &p_p
 #endif
 
 	String str;
-	Error err = OS::get_singleton()->execute(signtool_path, args, true, nullptr, &str, nullptr, true);
+	Error err = OS::get_singleton()->execute(signtool_path, args, &str, nullptr, true);
 	ERR_FAIL_COND_V(err != OK, err);
 
 	print_line("codesign (" + p_path + "): " + str);
diff --git a/platform/windows/os_windows.cpp b/platform/windows/os_windows.cpp
index 051b69e8d9..f0848ff880 100644
--- a/platform/windows/os_windows.cpp
+++ b/platform/windows/os_windows.cpp
@@ -410,24 +410,23 @@ String OS_Windows::_quote_command_line_argument(const String &p_text) const {
 	return p_text;
 }
 
-Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
+Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
 	String path = p_path.replace("/", "\\");
+	String command = _quote_command_line_argument(path);
+	for (const List<String>::Element *E = p_arguments.front(); E; E = E->next()) {
+		command += " " + _quote_command_line_argument(E->get());
+	}
 
-	if (p_blocking && r_pipe) {
-		String argss = _quote_command_line_argument(path);
-		for (const List<String>::Element *E = p_arguments.front(); E; E = E->next()) {
-			argss += " " + _quote_command_line_argument(E->get());
-		}
-
+	if (r_pipe) {
 		if (read_stderr) {
-			argss += " 2>&1"; // Read stderr too
+			command += " 2>&1"; // Include stderr
 		}
-		// Note: _wpopen is calling command as "cmd.exe /c argss", instead of executing it directly, add extra quotes around full command, to prevent it from stripping quotes in the command.
-		argss = _quote_command_line_argument(argss);
-
-		FILE *f = _wpopen((LPCWSTR)(argss.utf16().get_data()), L"r");
-		ERR_FAIL_COND_V(!f, ERR_CANT_OPEN);
+		// Add extra quotes around the full command, to prevent it from stripping quotes in the command,
+		// because _wpopen calls command as "cmd.exe /c command", instead of executing it directly
+		command = _quote_command_line_argument(command);
 
+		FILE *f = _wpopen((LPCWSTR)(command.utf16().get_data()), L"r");
+		ERR_FAIL_COND_V_MSG(!f, ERR_CANT_OPEN, "Cannot create pipe from command: " + command);
 		char buf[65535];
 		while (fgets(buf, 65535, f)) {
 			if (p_pipe_mutex) {
@@ -438,20 +437,40 @@ Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments,
 				p_pipe_mutex->unlock();
 			}
 		}
-
 		int rv = _pclose(f);
+
 		if (r_exitcode) {
 			*r_exitcode = rv;
 		}
-
 		return OK;
 	}
 
-	String cmdline = _quote_command_line_argument(path);
-	const List<String>::Element *I = p_arguments.front();
-	while (I) {
-		cmdline += " " + _quote_command_line_argument(I->get());
-		I = I->next();
+	ProcessInfo pi;
+	ZeroMemory(&pi.si, sizeof(pi.si));
+	pi.si.cb = sizeof(pi.si);
+	ZeroMemory(&pi.pi, sizeof(pi.pi));
+	LPSTARTUPINFOW si_w = (LPSTARTUPINFOW)&pi.si;
+
+	int ret = CreateProcessW(nullptr, (LPWSTR)(command.utf16().ptrw()), nullptr, nullptr, false, NORMAL_PRIORITY_CLASS & CREATE_NO_WINDOW, nullptr, nullptr, si_w, &pi.pi);
+	ERR_FAIL_COND_V_MSG(ret == 0, ERR_CANT_FORK, "Could not create child process: " + command);
+
+	WaitForSingleObject(pi.pi.hProcess, INFINITE);
+	if (r_exitcode) {
+		DWORD ret2;
+		GetExitCodeProcess(pi.pi.hProcess, &ret2);
+		*r_exitcode = ret2;
+	}
+	CloseHandle(pi.pi.hProcess);
+	CloseHandle(pi.pi.hThread);
+
+	return OK;
+};
+
+Error OS_Windows::create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id) {
+	String path = p_path.replace("/", "\\");
+	String command = _quote_command_line_argument(path);
+	for (const List<String>::Element *E = p_arguments.front(); E; E = E->next()) {
+		command += " " + _quote_command_line_argument(E->get());
 	}
 
 	ProcessInfo pi;
@@ -460,27 +479,15 @@ Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments,
 	ZeroMemory(&pi.pi, sizeof(pi.pi));
 	LPSTARTUPINFOW si_w = (LPSTARTUPINFOW)&pi.si;
 
-	Char16String modstr = cmdline.utf16(); // Windows wants to change this no idea why.
-	int ret = CreateProcessW(nullptr, (LPWSTR)(modstr.ptrw()), nullptr, nullptr, 0, NORMAL_PRIORITY_CLASS & CREATE_NO_WINDOW, nullptr, nullptr, si_w, &pi.pi);
-	ERR_FAIL_COND_V(ret == 0, ERR_CANT_FORK);
+	int ret = CreateProcessW(nullptr, (LPWSTR)(command.utf16().ptrw()), nullptr, nullptr, false, NORMAL_PRIORITY_CLASS & CREATE_NO_WINDOW, nullptr, nullptr, si_w, &pi.pi);
+	ERR_FAIL_COND_V_MSG(ret == 0, ERR_CANT_FORK, "Could not create child process: " + command);
 
-	if (p_blocking) {
-		WaitForSingleObject(pi.pi.hProcess, INFINITE);
-		if (r_exitcode) {
-			DWORD ret2;
-			GetExitCodeProcess(pi.pi.hProcess, &ret2);
-			*r_exitcode = ret2;
-		}
-
-		CloseHandle(pi.pi.hProcess);
-		CloseHandle(pi.pi.hThread);
-	} else {
-		ProcessID pid = pi.pi.dwProcessId;
-		if (r_child_id) {
-			*r_child_id = pid;
-		}
-		process_map->insert(pid, pi);
+	ProcessID pid = pi.pi.dwProcessId;
+	if (r_child_id) {
+		*r_child_id = pid;
 	}
+	process_map->insert(pid, pi);
+
 	return OK;
 };
 
diff --git a/platform/windows/os_windows.h b/platform/windows/os_windows.h
index 78258f132b..1a8791196b 100644
--- a/platform/windows/os_windows.h
+++ b/platform/windows/os_windows.h
@@ -136,7 +136,8 @@ public:
 	virtual void delay_usec(uint32_t p_usec) const override;
 	virtual uint64_t get_ticks_usec() const override;
 
-	virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking = true, ProcessID *r_child_id = nullptr, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	virtual Error execute(const String &p_path, const List<String> &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	virtual Error create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id = nullptr) override;
 	virtual Error kill(const ProcessID &p_pid) override;
 	virtual int get_process_id() const override;